[PATCH] D143681: [llvm][test] restrict 2 GVN tests to just test GVN (NFC)

Tue Feb 14 02:07:20 PST 2023

nikic added inline comments.

================
Comment at: llvm/test/Transforms/GVN/no_speculative_loads_with_asan.ll:6

+; load of %i8 is redundant wrt. load of %i3.
 define i32 @TestNoAsan() {
----------------
nickdesaulniers wrote:
> Q: isn't `%i8` based on `gep ptr %i, 1` while `%i3` is based on `get ptr %i, 2`? Aren't those different addresses? Or is it because it's a 16b load (so the two adjacent 8bits)?
`%i8` is an out of bounds load which can be folded to poison (which then allows eliding the phi).

================
Comment at: llvm/test/Transforms/NewGVN/no_speculative_loads_with_asan.ll:19-20
+; CHECK-NEXT:    [[TMP8:%.*]] = bitcast i8* [[TMP7]] to i16*
+; CHECK-NEXT:    [[TMP9:%.*]] = load i16, i16* [[TMP8]], align 2
+; CHECK-NEXT:    [[TMP10:%.*]] = sext i16 [[TMP9]] to i32
+; CHECK-NEXT:    br label [[TMP11]]
----------------
nickdesaulniers wrote:
> I think these two instructions show that this whole test is kind of pointless IMO.
> 
> It's a copy of llvm/test/Transforms/GVN/no_speculative_loads_with_asan.ll, but using `newgvn` rather than `gvn` (unless I messed something up by converting this from `-O3`.
> 
> The `@TestNoAsan` case is the same as the `@TestAsan` case, so it just shows that NewGVN cannot elide redundant non-local loads. Am I understanding correctly?
Yes, load folding in NewGVN is incomplete. Your new test it fine (the previous one was broken, because it did not actually test newgvn).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143681