[PATCH] D50636: [DAGCombiner] Add X / X -> 1 & X % X -> 0 folds.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added inline comments.


================
Comment at: test/CodeGen/SystemZ/pr32372.ll:18
   store i16 -3825, i16* undef
-  %L5 = load i8, i8* %0
+  %L5 = load volatile i8, i8* %0
   %B9 = urem i8 %L5, %L
----------------
craig.topper wrote:
> jonpa wrote:
> > RKSimon wrote:
> > > Tried again against bleeding edge trunk and I still see this change and without the volatile (which prevents the loads being combined) the checks reduce to:
> > > ```
> > > define void @pr32372(i8*) {
> > > ; CHECK-LABEL: pr32372:
> > > ; CHECK:       # %bb.0: # %BB
> > > ; CHECK-NEXT:    mvhhi 0(%r1), -3825
> > > ; CHECK-NEXT:  .LBB0_1: # %CF251
> > > ; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
> > > ; CHECK-NEXT:    j .LBB0_1
> > > ```
> > Yes, I see the change after updating the test, but also note that this updated test passes also without the patch... So I am not sure why the test is changed if the patch does not affect it...
> I think this patch affects the original version of the test. Without the volatile the two loads get CSEd to the same SDNode. This causes the same SDNode to be both operands of the urem. This patch would simplify that to 0. Simon added the volatile to prevent the loads from being CSEd. This way the urem won't get affected by this patch. So the new version should pass on trunk as well.
Ah, I see. Thank you for the explanation.


Repository:
  rL LLVM

https://reviews.llvm.org/D50636