HomePhabricator

[CodeGen] remove instcombine from codegen tests; NFC

Authored by spatel on Jun 8 2021, 12:12 PM.

Description

[CodeGen] remove instcombine from codegen tests; NFC

The FileCheck lines in these files are auto-generated and complete,
so there's very little upside (less CHECK lines) from running
-instcombine on them and violating the expected test layering
(optimizer developers shouldn't have to be aware of clang tests).

Running opt passes like this makes it harder to make changes such as:
D93817

Details

Committed
spatelJun 8 2021, 12:31 PM
Parents
rG2a5afb466553: [CMake][Fuchsia] Use PIC for Fuchsia runtimes
Branches
Unknown
Tags
Unknown

Event Timeline

abhinavgaba added inline comments.
/clang/test/CodeGen/arm-bf16-convert-intrinsics.c
13–16

Could you please limit this to the new pass manager? It fails with the legacy pass manager without this patch:

diff --git a/clang/test/CodeGen/arm-bf16-convert-intrinsics.c b/clang/test/CodeGen/arm-bf16-convert-intrinsics.c
index 7c65f7270ab7..68c71703469b 100644
--- a/clang/test/CodeGen/arm-bf16-convert-intrinsics.c
+++ b/clang/test/CodeGen/arm-bf16-convert-intrinsics.c
@@ -13,7 +13,7 @@
 // RUN: %clang_cc1 \
 // RUN:   -triple armv8.6a-arm-none-eabi -target-feature +neon \
 // RUN:   -target-feature +bf16 -mfloat-abi softfp \
-// RUN:   -disable-O0-optnone -emit-llvm -o - %s \
+// RUN:   -disable-O0-optnone -emit-llvm -fno-legacy-pass-manager -o - %s \
 // RUN:   | opt -S -mem2reg \
 // RUN:   | FileCheck --check-prefixes=CHECK,CHECK-A32-SOFTFP %s

+1 to limiting it to new pass manager. (good spot btw, I was very confused why this was failing for me)

You get a different order of alloca statements when using the legacy PM.

+1 to limiting it to new pass manager. (good spot btw, I was very confused why this was failing for me)

You get a different order of alloca statements when using the legacy PM.

Thanks for letting me know - see if this got all of the problems RUN lines:
cc86b87a5700

I didn't see any bot fail mails (but that might be intentional?).
Does it seem odd that the order of alloca would be different (inside of mem2reg?) between pass managers? Maybe there's some difference in initialization that was not intended.

Opened https://bugs.llvm.org/show_bug.cgi?id=50659

Could be that the New PM does a better job of naming things and we just stick with the workaround. We're not going to miss anything important with regard to the actual bfloat functionality because of it.

I didn't see any bot fail mails (but that might be intentional?).

I think some cmake caches got stuck with the new PM off somehow, see the last comment here: https://reviews.llvm.org/D95380.

My local build folders have been around long enough and cmake didn't apply the new default when I just re-ran it instead of doing a clean build. So I had it OFF, unintentionally.