[llvm-commits] [PATCH] Defer some shl transformations from InstCombine to DAGCombine

[Chandler Carruth]

The patch looks good to me. A couple of minor comments about testing this...

Because it is a phase-ordering kind of problem, I worry a lot about
regressions. Specifically, if we ever get some other pass that nukes your
"silly function", will we stop testing that this works?

I wonder, could we add a direct test that SCEV can undrestand this pattern
when InstCombine runs first? That would seem more targeted.

Can we add a test for the LSR usage as well? Maybe using the same pattern
as above?

Finally, a nit pick on the patch:

diff --git a/test/Transforms/InstCombine/shift.ll
b/test/Transforms/InstCombine/shift.ll
index 52310e3..21c0a33 100644
--- a/test/Transforms/InstCombine/shift.ll
+++ b/test/Transforms/InstCombine/shift.ll
>>> snip <<<
+;; This is allowed only when %B is single-use.

Should this be "deferred to DAG combining" as with the other comments in
this test file?

Thanks,
-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120418/c1d0327e/attachment.html>