[Clang][Misc] Fix fragile test

[atmnpatel]

Summary

This test has %clang in the run line when it should have %clang_cc1. When built in release mode, %clang will discard labels for blocks, so it is unable to check for block labels.

[MaskRay]

Do you know why some bots don't emit entry:?

[atmnpatel]

I can't say that I know with any certainty, I'm too inexperienced. This failure was missed by me locally, the pre-merge bot, and by most of the buildbots other than the ones I mentioned above. I have no idea under what configuration of CMake options will clang not emit this particular case of a label for a BB.

[MaskRay]

You can check the CMake configurations of these bots.

https://github.com/llvm/llvm-zorg buildbot/osuosl/master/config/builders.py

I think it'd be good to know why. It may be a bug.

[fhahn]

In D93952#2475526, @atmnpatel wrote:

I can't say that I know with any certainty, I'm too inexperienced. This failure was missed by me locally, the pre-merge bot, and by most of the buildbots other than the ones I mentioned above. I have no idea under what configuration of CMake options will clang not emit this particular case of a label for a BB.

The problem is that this test is using %clang instead of %clang_cc1. Executing clang directly will pass -fdiscard-value-names when built with release mode; all blocks/values will be numbered, instead of explicitly named. It would be better to update the test to use %clang_cc1, to make it independent on flags passed depending on configuration of clang.

[atmnpatel]

Ah I see, I was never able to find clear documentation on what exactly the -cc1 flag does other than the conceptual description. This should fix it right?

[MaskRay]

In D93952#2475762, @atmnpatel wrote:

Ah I see, I was never able to find clear documentation on what exactly the -cc1 flag does other than the conceptual description. This should fix it right?

-cc1 options are implementation details and are unstable. Driver options need to be more stable and usually cannot be removed.

Ah I forgot that the driver option's default -f[no-]discard-value-names is dependent on -DLLVM_ENABLE_ASSERTIONS.

Can you also update the summary of this Diff and add a file-level comment to the test?

[atmnpatel]

Better?

[MaskRay]

clang/test/Misc/loop-opt-setup.c

1–2

Most tests use %clang_cc1 for relatively stable flag behavior so this comment is not needed.

The comment should mention the purpose of this test and probably why -O1 is used.

clang/test/Misc/loop-opt-setup.c

 // RUN: %clang -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
 // RUN: %clang -std=c99 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s --check-prefix C99
 
 extern int a[16];
 int b = 0;
 int foo(void) {
 #pragma unroll
   for (int i = 0; i < 16; ++i)
     a[i] = b += 2;
   return b;

 
 // Check br i1 to make sure the loop is gone, there will still be a label branch for the infinite loop.
 // In C99, there was no forward progress requirement, so we expect the infinite loop to still exist,
 // but for C11 and onwards, the infinite loop can be deleted.
 // CHECK-LABEL: Helper
 // C99: br label
 // C99-NOT: br i1
 // C99: br label
-// CHECK: entry:
 // CHECK-NOT: br i1
-// CHECK-NEXT: ret void
+// CHECK: ret void