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

[Marcin Kościelnicki via llvm-commits]

koriakin created this revision.
koriakin added reviewers: hfinkel, resistor.
koriakin added a subscriber: llvm-commits.
koriakin set the repository for this revision to rL LLVM.

visitAND, when folding and (load) forgets to check the load is UNINDEXED,
happily folding the updated address output on the following testcase:

target datalayout = "e-m:e-i64:64-n32:64"
target triple = "powerpc64le-unknown-linux-gnu"

%typ = type { i32, i32 }

define signext i32 @_Z8access_pP1Tc(%typ* %p, i8 zeroext %type) {
  %b = getelementptr inbounds %typ, %typ* %p, i64 0, i32 1
  %1 = load i32, i32* %b, align 4
  %2 = ptrtoint i32* %b to i64
  %3 = and i64 %2, -35184372088833
  %4 = inttoptr i64 %3 to i32*
  %_msld = load i32, i32* %4, align 4
  %zzz = add i32 %1,  %_msld
  ret i32 %zzz
}

Avoid the issue by disallowing transforming indexed loads entirely.

I've found a few more places that currently neglect to check for
indexed load, and tightened them up as well, but I don't have test
cases for them.  In fact, they might not be triggerable at all,
at least with current targets.  Still, better safe than sorry.

Repository:
  rL LLVM

http://reviews.llvm.org/D19202

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  test/CodeGen/PowerPC/2016-04-17-combine.ll

Index: test/CodeGen/PowerPC/2016-04-17-combine.ll
===================================================================
--- /dev/null
+++ test/CodeGen/PowerPC/2016-04-17-combine.ll
@@ -0,0 +1,26 @@
+; RUN: llc <%s | FileCheck %s
+target datalayout = "e-m:e-i64:64-n32:64"
+target triple = "powerpc64le-unknown-linux-gnu"
+
+; PR27390 crasher
+
+%typ = type { i32, i32 }
+
+; On release builds, it doesn't crash, spewing nonsense instead.
+; To make sure it works, check that and is still alive.
+; CHECK: and
+; Also, in release, it emits a COPY from a 32-bit register to
+; a 64-bit register, which happens to be emitted as cror [!]
+; by the confused CodeGen.  Just to be sure, check there isn't one.
+; CHECK-NOT: cror
+; Function Attrs: uwtable
+define signext i32 @_Z8access_pP1Tc(%typ* %p, i8 zeroext %type) {
+  %b = getelementptr inbounds %typ, %typ* %p, i64 0, i32 1
+  %1 = load i32, i32* %b, align 4
+  %2 = ptrtoint i32* %b to i64
+  %3 = and i64 %2, -35184372088833
+  %4 = inttoptr i64 %3 to i32*
+  %_msld = load i32, i32* %4, align 4
+  %zzz = add i32 %1,  %_msld
+  ret i32 %zzz
+}
Index: lib/CodeGen/SelectionDAG/SelectionDAG.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -6962,6 +6962,8 @@
                                                   int Dist) const {
   if (LD->isVolatile() || Base->isVolatile())
     return false;
+  if (LD->isIndexed() || Base->isIndexed())
+    return false;
   if (LD->getChain() != Base->getChain())
     return false;
   EVT VT = LD->getValueType(0);
Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -962,7 +962,8 @@
 SDValue DAGCombiner::PromoteOperand(SDValue Op, EVT PVT, bool &Replace) {
   Replace = false;
   SDLoc dl(Op);
-  if (LoadSDNode *LD = dyn_cast<LoadSDNode>(Op)) {
+  if (ISD::isUNINDEXEDLoad(Op.getNode())) {
+    LoadSDNode *LD = cast<LoadSDNode>(Op);
     EVT MemVT = LD->getMemoryVT();
     ISD::LoadExtType ExtType = ISD::isNON_EXTLoad(LD)
       ? (TLI.isLoadExtLegal(ISD::ZEXTLOAD, PVT, MemVT) ? ISD::ZEXTLOAD
@@ -1166,6 +1167,9 @@
   if (!LegalOperations)
     return false;
 
+  if (!ISD::isUNINDEXEDLoad(Op.getNode()))
+    return false;
+
   EVT VT = Op.getValueType();
   if (VT.isVector() || !VT.isInteger())
     return false;
@@ -3095,8 +3099,8 @@
   // more cases.
   if ((N0.getOpcode() == ISD::EXTRACT_VECTOR_ELT &&
        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())) {
     LoadSDNode *Load = cast<LoadSDNode>( (N0.getOpcode() == ISD::LOAD) ?
                                          N0 : N0.getOperand(0) );
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D19202.54013.patch
Type: text/x-patch
Size: 3003 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160417/4010ce81/attachment.bin>