[llvm-commits] [PATCH, RFC] Fix problem with combining a load and shift

[Bill Schmidt]

This patch addresses an incorrect transformation in the DAG combiner.

The included test case is derived from one of the GCC compatibility tests.
The problem arises after the selection DAG has been converted to type-legalized
form.  The combiner first sees a 64-bit load that can be converted into a
pre-increment form.  The original load feeds into a SRL that isolates the
upper 32 bits of the loaded doubleword.  This looks like an opportunity for
DAGCombiner::ReduceLoadWidth() to replace the 64-bit load with a 32-bit load.

However, this transformation is not valid, as the replacement load is not
a pre-increment load.  The pre-increment load produces an extra result,
which feeds a subsequent add instruction.  The replacement load only has
one result value, and this value is propagated to all uses of the pre-
increment load, including the add.  Because the add is looking for the
second result value as its operand, it ends up attempting to add a constant
to a token chain, resulting in a crash.

So the patch simply disables this transformation for any load with more than
two result values.

I've tentatively placed the new test case in CodeGen/PowerPC for want of
a better location.  Since the IR has been type-legalized for the target
when the bug occurs, the bug may occur only on certain targets.  The test
just checks whether the code compiles successfully, so it is appropriate
to execute on all targets.

Any concerns with this approach?  Is this ok for mainline?

Thanks!

Bill
-------------- next part --------------
A non-text attachment was scrubbed...
Name: combine-2013-01-09.patch
Type: text/x-patch
Size: 3170 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130109/23bc0e83/attachment.bin>