[PATCH] D128364: [InstCombine] Look through more casts when folding memchr and memcmp

Martin Sebor via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 28 15:02:09 PDT 2022


msebor added a comment.

Thanks for the careful review!



================
Comment at: llvm/test/Transforms/InstCombine/memchr-10.ll:21
+; CHECK-LABEL: @call_memchr_ap5_c_1_eq_a(
+; CHECK-NEXT:    ret i1
+;
----------------
nikic wrote:
> Incomplete check line?
It's intentional and documented above: the return value doesn't matter (i.e., it's indeterminate here).   I didn't want to encode an expectation one way or the other.


================
Comment at: llvm/test/Transforms/InstCombine/memchr-9.ll:317
+; CHECK-NEXT-BE:    ret i8* getelementptr (i8, i8* bitcast (%union.U* @u to i8*), i64 5)
+; CHECK-NEXT-LE:    ret i8* bitcast (i32* getelementptr inbounds (%union.U, %union.U* @u, i64 0, i32 0, i64 1) to i8*)
+;
----------------
nikic wrote:
> These check lines aren't correct and are probably just getting ignored. You need to pass something like `--check-prefixes=CHECK,BE` and `--check-prefixes=CHECK,LE` and regenerate check lines.
Right.  You do have a good eye for detail!


================
Comment at: llvm/test/Transforms/InstCombine/strcall-no-nul.ll:142
+;
+; XFAIL-CHECK-NEXT: ret i8*, null
+  %p5 = getelementptr [5 x i8], [5 x i8]* @a5, i32 0, i32 5
----------------
nikic wrote:
> This check line doesn't make a lot of sense ... the syntax is incorrect (and the incorrect syntax is repeated in the cases below) and the ret wouldn't make sense here anyway (because strlen returns an integer not a pointer). The TODO also seems outdated, as this already folds to 0.
I've fixed the `strlen` typo.

I hand-wrote all these lines and I realize not all of them are entirely correct.  My main goal was to more prominently mark up the expected failures, but I was also hoping that `llvm-lit` would pick them up somehow and indicate when there are expected failures in the test.  I've removed the rest of the `XFAIL-` lines from the final commit but, for future reference, is there a way to do what I want?  (I couldn't find an example in the test suite that I didn't add myself.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128364



More information about the llvm-commits mailing list