i1 promotion issue (again)

Background: The Cell SPU does not have condition registers in the normal sense. It fits the “zero or negative one” model, preferably with an i32 register, which is what getSetCCResultType() will return.

Problem: LegalizeTypes promotes i1 to i8 via an i32 setcc, i.e., the generated type legalization is:

(i8:truncate (i32:setcc i32:lhs, i32:rhs, ch:cond))

How do I keep DAGTypeLegalizer::PromoteIntRes_SETCC() from inserting the truncate without blowing all other type promotion out of the water (having everything promoted up to i32 as a consequence)? Would it be permissible to add a virtual function to TargetLowering such that the target can effectively say, “Really, the setcc is legal, no need to truncate!”?

The alternative is to do fairly deep inspection of brconds to eliminate the truncate, which eventually gets expanded into:

(i8:sext_in_reg (i8:truncate (i32:setcc …)))

-scooter

Have you tried implementing computeMaskedBitsForTargetNode for your
setcc nodes? If you have, I think DAGCombiner should take care of the
necessary simplification.

-Eli

Umm, make that ComputeNumSignBitsForTargetNode.

-Eli

Hi Scott,

Problem: LegalizeTypes promotes i1 to i8 via an i32 setcc, i.e., the
generated type legalization is:

  (i8:truncate (i32:setcc i32:lhs, i32:rhs, ch:cond))

How do I keep DAGTypeLegalizer::PromoteIntRes_SETCC() from inserting
the truncate without blowing all other type promotion out of the
water (having everything promoted up to i32 as a consequence)? Would
it be permissible to add a virtual function to TargetLowering such
that the target can effectively say, "Really, the setcc is legal, no
need to truncate!"?

since CellSPU says that i1 should be promoted into an i8, that's
exactly what's being done. The SetCC promotion logic doesn't know
who's using the value - it can't just change the type randomly: i8
is expected by users, as far as it knows, so it must produce an i8.

The situation is quite different for users of values though. It sounds
to me like the real question is: why is the brcond condition being promoted
from i1 to i8 and not to i32? When promoting an operand you can change
the type promoted to without any problem. I've got nothing against having
the brcond logic call getSetCCResultType to find out what type should be
used. The only problem is that getSetCCResultType takes an argument, and
what should be passed? You could pass i1, but on CellSPU it would pass
i1 right back :slight_smile: There's a solution to that: modify getSetCCResultType
so it doesn't take an argument. After all, only one platform uses it:
CellSPU :wink:

The alternative is to do fairly deep inspection of brconds to
eliminate the truncate, which eventually gets expanded into:

  (i8:sext_in_reg (i8:truncate (i32:setcc ...)))

I think there's no problem: you just do a zext of the i8 value, giving:
zext to i32 of truncate to i8 of i32 <- known to be zero extended
from i1. The DAG combiner should drop all the extensions and truncations.

Ciao,

Duncan.

Hi Eli,

Have you tried implementing computeMaskedBitsForTargetNode for your
setcc nodes? If you have, I think DAGCombiner should take care of the
necessary simplification.

he doesn't need to: the DAG combiner knows all about SetCC values,
and should simplify this already.

Ciao,

Duncan.

Oh, this is ISD::SETCC? SelectionDAG::ComputeNumSignBits doesn't
currently know how to handle it, but that should be easy to fix.

-Eli

ComputeNumSignBits() is never called.

Moreover, Cell SPU has to custom lower truncates in a specific way to preserve register uniformity (scalar and vector representation is one and the same, or, put another way, the scalar and vector registers are the same register.) Consequently, I can't trap (truncate (setcc ...)) patterns easily because the truncate is being custom lowered. Hence, DAGCombiner isn't doing me much good.

-scooter

That seems very weird... the DAG combiner will always call
ComputeNumSignBits on any sext_in_reg. Also, the only way I can think
of that you're getting an sext_in_reg in this situation is that
PromoteIntOp_BRCOND is creating it, and it should call
ComputeNumSignBits in that case.

Also, I just tried out a branch with the CellSPU backend in trunk
LLVM, and I didn't see the sext_in_reg; is there something I'm
missing, or are there some relevant changes in your tree you haven't
committed yet?

-Eli

Hi,

Moreover, Cell SPU has to custom lower truncates in a specific way to
preserve register uniformity (scalar and vector representation is one
and the same, or, put another way, the scalar and vector registers
are the same register.) Consequently, I can't trap (truncate
(setcc ...)) patterns easily because the truncate is being custom
lowered. Hence, DAGCombiner isn't doing me much good.

during type legalization custom lowering is only called for nodes
with illegal types. Since the truncate in question (i32 -> i8)
only uses legal types, the CellSPU custom lowering code will not
be called on it during type legalization. When type legalization
is done, the DAG combiner runs. This should clean things up,
removing any pointless truncate-extend stuff. As this point the
operation legalizer runs, and any remaining truncate nodes will be
custom lowered. So I don't see why there should be a problem.

Ciao,

Duncan.