[PATCH] D19202: [PR27390] [CodeGen] Reject indexed loads in CombinerDAG.

[Marcin Kościelnicki via llvm-commits]

koriakin planned changes to this revision.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3102
@@ -3097,3 +3101,3 @@
        N0.getValueSizeInBits() == N0.getOperand(0).getScalarValueSizeInBits() &&
-       N0.getOperand(0).getOpcode() == ISD::LOAD) ||
-      N0.getOpcode() == ISD::LOAD) {
+       ISD::isUNINDEXEDLoad(N0.getOperand(0).getNode()))||
+      ISD::isUNINDEXEDLoad(N0.getNode())) {
----------------
hfinkel wrote:
> It is not clear this is necessary; the code below explicitly handles indexed loads:
> 
>       if (Load->getExtensionType() == ISD::EXTLOAD) {
>         NewLoad = DAG.getLoad(Load->getAddressingMode(), ISD::ZEXTLOAD,
>                               Load->getValueType(0), SDLoc(Load),
>                               Load->getChain(), Load->getBasePtr(),
>                               Load->getOffset(), Load->getMemoryVT(),
>                               Load->getMemOperand());
>         // Replace uses of the EXTLOAD with the new ZEXTLOAD.
>         if (Load->getNumValues() == 3) {
>           // PRE/POST_INC loads have 3 values.
>           SDValue To[] = { NewLoad.getValue(0), NewLoad.getValue(1),
>                            NewLoad.getValue(2) };
>           CombineTo(Load, To, 3, true);
Ah, indeed it does.  So it's OK to allow indexed loads here, but we'll need to check if N0.getResNo() is 0.  I'll update the patch.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:6965
@@ -6964,1 +6964,3 @@
     return false;
+  if (LD->isIndexed() || Base->isIndexed())
+    return false;
----------------
hfinkel wrote:
> Why?
The only caller (DAGCombiner::CombineConsecutiveLoads) doesn't support indexed loads, and I don't see any other place where they're checked for, but I don't actually have any example code that could trigger a problem here.


Repository:
  rL LLVM

http://reviews.llvm.org/D19202