[PATCH] D24244: SHL instruction crash in CodeGen - Fixed

[Simon Pilgrim via llvm-commits]

RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM with a few minors


================
Comment at: ../lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:1080
@@ +1079,3 @@
+        // to update the parent node.
+        if (SAO != Node->getOperand(1))
+          NewNode = DAG.UpdateNodeOperands(Node, Node->getOperand(0), SAO);
----------------
delena wrote:
> RKSimon wrote:
> > So are you saying that if SAO == Node->getOperand(1) then its guaranteed to be already legal? Do we need to assert this?
> No. I say that SAO node will legalized in the next round. Legalization of more than one node together may crash. 
OK, please make that more clear in the comment.

================
Comment at: ../test/CodeGen/X86/shl-crash-on-legalize.ll:10
@@ +9,3 @@
+ at _ZN8struct_210member_2_3E = external local_unnamed_addr global i64, align 8
+ at var_366 = external local_unnamed_addr global i8, align 1
+
----------------
var_366 doesn't appear to be referenced.

================
Comment at: ../test/CodeGen/X86/shl-crash-on-legalize.ll:31
@@ +30,3 @@
+  ret i32 %shl420
+}
+
----------------
Could rename the variables to make it more readable?


Repository:
  rL LLVM

https://reviews.llvm.org/D24244