Made casts explicit in the IRPrinter and StmtToHtml

[mcourteaux]

Now printing as cast<type>(value)

[abadams]

I worry this is going to make large bits of fixed-point IR harder to read. What was ambigious about it before?

[alexreinking]

I worry this is going to make large bits of fixed-point IR harder to read. What was ambigious about it before?

Before, it used to present IntImm literals the same way as Casts... but that's confusing and even counterproductive when trying to extract a test case from a printed output.

[abadams]

Immediates were (type)value and casts were type(value). I.e. casts use the type as a function.

[abadams]

If that's too subtle of a difference, perhaps we should change how immediates print so that they look less like C-style casts? In large fixed-point Exprs you want to visually minimize the amount of real-estate the cast nodes take up, so that you can focus on the actual operators. This is why the ConciseCasts namespace exists for writing Exprs.

[mcourteaux]

One other reason is that several nodes print their resulting type in front of them.

For example VectorReduce looks totally like a cast, but is actually a crucial part of the very definition of the reduce operator used (to determine the reduction factor).

Halide/src/IRPrinter.cpp

Lines 1478 to 1479 in 7e40343

	void IRPrinter::visit(const VectorReduce *op) {
	stream << typep(op->type);

Sometimes, Loads do the same to indicate what the type of the load is. Then there is no distinction anymore between a Cast(Load()) and an annotated Load():

Halide/src/IRPrinter.cpp

Lines 995 to 997 in 7e40343

	if (!known_type.contains(op->name)) {
	stream << typep(op->type);
	}

Variables do that all the time:

Halide/src/IRPrinter.cpp

Lines 818 to 820 in 7e40343

	} else {
	stream << typep(op->type);
	}

Calls do it as well:

Halide/src/IRPrinter.cpp

Lines 1067 to 1077 in 7e40343

	if (!known_type.contains(op->name) &&
	(op->type != Int(32))) {
	if (op->type.is_handle()) {
	stream << type(op->type); // Already has parens
	} else {
	stream << typep(op->type);
	}
	}
	openf(op->name.c_str());
	print_list(op->args);
	closef();

---

Perhaps we just use different syntax for casts: perhaps <type>value?

Observe here that it's clear now that the abs() result is not being casted, but just annotating the return type.

[mcourteaux]

Immediates were (type)value and casts were type(value). I.e. casts use the type as a function.

That's even backwards from C, I just realized. Perhaps also confusing on it's own? I prefer C-style casts, and constructor-style literals:

• Immediate uint64: uint64(120)
• Cast to uint64: (uint64)120

Which is exactly backwards to what we have right now.

[abadams]

(t)foo declares that the type of expression foo is t, and this applies to constants, calls, vectorreduce, vars, etc. In other languages this is written foo : t

In C++ you can use primitive types as constructors, so either way around makes a cast.

[mcourteaux]

(t)foo declares that the type of expression foo is t, and this applies to constants, calls, vectorreduce, vars, etc. In other languages this is written foo : t

In C++ you can use primitive types as constructors, so either way around makes a cast.

Hm, I see. So nothing was ambiguous. I think @alexreinking and I were both fooled by the swapped notations compared to C++. What looks like a cast in C/C++ is a type-annotation in Halide IR, and what looks like a constructor-call in C++ is a cast in Halide IR.

[alexreinking]

I think two core contributors being confused about a notation is reason enough to change it.

[mcourteaux]

We can already take away quite some confusion by just introducing suffixes from the non-int32 literals. I propose _u8, _i8, _u16, _i16, etc. I think this is a clarity improvement, regardless of what we do with casts. @abadams thoughts?

I'm not disliking this (cast is still with my <type> experiment syntax):

[abadams]

I suspect the reason for the confusion was that cast<uint8_t>(17) prints as (uint8_t)17, because the cast operator eagerly folds into an immediate of a different type when given a constant. It's not a cast node.

[alexreinking]

I suspect the reason for the confusion was that cast<uint8_t>(17) prints as (uint8_t)17, because the cast operator eagerly folds into an immediate of a different type when given a constant. It's not a cast node.

Yes, that's exactly it. Using cast syntax without a corresponding cast node is the crux of the confusion.

[abadams]

Suffixes for literals are certainly an option, but that doesn't address vars and calls. I would also be supportive of shortening printing of the cast operators to the ones in ConciseCasts so that it looks more like user-written Halide (which I think was the rationale behind the PR) but without making the IR more verbose. We'd have to extend it for vector types though, so it wouldn't be literally copy-pasteable in all contexts. Consider Cast::make(Int(8, 4), Ramp::make(Variable::make(Int(16), x), 3, 4))

Right now we have:
int8x4(ramp((int16)x, (int16)3, 4))

Which personally looks fine to me, but maybe that's just familiarity. The type applied as a function is a cast, the ones in parens before an Expr are declarations of type. What do we want this to be?

i8x4(ramp((int16)x, 3_i16, 4)
int8x4(ramp(x : int16, 3 : int16, 4)
cast<int8x4>(ramp((int16)x, (int16)3, 4)
i8x4(ramp((i16)x, (i16)3, 4)
cast<i8x4>(ramp((i16)x, (i16)3, 4)
i8x4(ramp(i16 x, i16 3, 4)

I don't want to change it unless it's to something significantly better though, because this is user-facing and familiarity is also important for existing users.

[alexreinking]

This is in design flux still, so I removed my "Approve" until we reach some sort of consensus.

My MVP is "Doesn't look like a cast in the absence of a cast".

[mcourteaux]

More thought went into this. I like the next two forms:

let x : i16 = ...
i8x4(ramp(x, 3_i16, 4))

Concise, and functional-style cast: casting is a function, so you can see it does something. Suffixes get rid of a lot of clutter. Example:

Or:

let x : i16 = ...
cast<i8x4>(ramp(x, 3_i16, 4))

More explicit, more clear.

[alexreinking]

Apparently re-requesting doesn't clear the old approval.

[alexreinking]

And neither does submitting a "Comment" review! Gah! Bad UX...

[mcourteaux]

Commited this style, as it's the closest to what we had, but just a bit clearer with the Let/LetStmt, and with the suffixes:

let x : i16 = ...
i8x4(ramp(x, 3_i16, 4))