[PATCH] D109283: [InstCombine] ror/rol(X, RotAmt) == C --> X == rol/ror(C, RotAmt) (PR51567)

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 6 08:55:56 PDT 2021


spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:3238
+
+      const APInt *RotAmt;
+      // ror(X, RotAmt) == C --> X == rol(C, RotAmt)
----------------
Calling this "RotC" or "RotAmtC" makes it more obvious that we are matching a constant.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-rotate.ll:115
+;
+  %f = tail call i8 @llvm.fshl.i8(i8 %x, i8 %x, i8 28)
+  %r = icmp eq i8 %f, 2
----------------
The over-shifting isn't relevant - please reduce to a valid amount (for example 28 -> 4) in this test and others.
But don't use "4" with a bitwidth of 8 either because that makes the test ambiguous about whether it is rotating in the wrong direction.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-rotate.ll:150
+
+define <2 x i1> @rol_eq_cst_undef(<2 x i5> %x) {
+; CHECK-LABEL: @rol_eq_cst_undef(
----------------
xbolva00 wrote:
> Enough tests?
We're missing a fshl with ne. Also at least one extra-use test should be here.
Please pre-commit baseline tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109283/new/

https://reviews.llvm.org/D109283



More information about the llvm-commits mailing list