[PATCH] D48789: [X86] Replace (32/64 - n) shift amounts with (neg n) since the shift amount is masked in hardware

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:2758
+  // Recapture operand 1 so we can delete it if its unused.
+  CurDAG->UpdateNodeOperands(N, N->getOperand(0), NewShiftAmt);
+
----------------
niravd wrote:
> You should overwrite N with the return value of the update in case the update is CSEd with another node, e.g., we are selecting in a block that has both the pre-optimized and post-optimized versions. Actually, we should probably add a test case for this.
> 
> N = CurDAG->UpdateNodeOperands(N, N->getOperand(0), NewShiftAmt);
I'm having a hard time getting the topological sort to give me an order that can cause this to happen. We're always doing isel on the post-optimized version i explicitly put in the IR first. There are more nodes in that path and that seems to be making it so that the pre-optimized path gets fully sorted into the topological order before the post-optimized side does.


https://reviews.llvm.org/D48789