[FileCheck] Disallow unused prefixes in llvm/test/Analysis

[mtrofin]

Summary

This is achieved through a substitution of FileCheck in lit.cfg.py,
where we explicitly set -allow-unused-prefixes to false.

We also introduce a %FileCheckWithUnusedPrefixes% substitution that can
be used in those cases where we want to allow unused prefixes, even if
the folder policy is to disallow them.

[MaskRay]

llvm/test/lit.cfg.py

87

Does ('FileCheck', 'FileCheck --allow-unused-prefixes=true') work?

You may take a look at some tools like opt,llc,llvm-mc,etc. They expand to the absolute paths. It is possible that FileCheck can use the same trick, but expand with another argument, so that you don't have to update the tests.

[mtrofin]

llvm/test/lit.cfg.py

87

The intent is to have a way to set --allow-unused-prefixes=false under whole directories, and offer a "way out" for the files where we want to allow it. This way, we can "seal" to the desired behavior whole directories after they are fixed.

This patch does that for llvm/test/Analysis.

We still don't want to blanket-allow the undesirable behavior for entire directories.

[jhenderson]

Looks okay, but I'm not really up to speed on the implications of lit substitutions, so it would be better for someone else to review if possible. In the ideal world, we'll revert this and any similar patches and just use the option explicitly, once we've fixed all the tests and changed the default to reduce obfuscation, in my opinion, but I'm not sure it's a big issue.

llvm/test/Analysis/lit.local.cfg

6

9

Nit: need new line at EOF.

[RKSimon]

So I'm clear - the final patch is just the lit.local.cfg changes? AFAICT the changes to arith-* are just examples that aren't actually needed

[mtrofin]

In D91275#2390594, @jhenderson wrote:

Looks okay, but I'm not really up to speed on the implications of lit substitutions, so it would be better for someone else to review if possible. In the ideal world, we'll revert this and any similar patches and just use the option explicitly, once we've fixed all the tests and changed the default to reduce obfuscation, in my opinion, but I'm not sure it's a big issue.

Right - when it's all done, meaning either prefixes are removed or test maintainers believe unused ones are intentional, we can:

• flip the default
• undo this change
• s/%FileCheckWithUnusedPrefixes%/--allow-unused-prefixes=true/g

[mtrofin]

In D91275#2390691, @RKSimon wrote:

So I'm clear - the final patch is just the lit.local.cfg changes? AFAICT the changes to arith-* are just examples that aren't actually needed

Not sure what you mean by 'final' - this is a patch to "stop the bleeding" in test/Analysis.

Happy to address the 2 arith-* cases if that's the case - I'd still leave the %FileCheckWithUnusedPrefixes% so we can use them elsewhere - i.e. for scripted fixups (with a "FIXME: is this intended?") for folders that don't appear to be maintained, or for folders where this is the intent (without the FIXME there, of course).

[RKSimon]

Sorry - I hadn't realised that some of the x86 costmodel tests were using the --allow-unused-prefixes flag - I'll see if I can drop them.

[mtrofin]

missing newline

[RKSimon]

The arith-fp.ll + arith-fma.ll changes shouldn't be needed after rGe11195d0a93239

[mtrofin]

rebase

[mtrofin]

Addressed comments

llvm/test/Analysis/lit.local.cfg

9

done

[mtrofin]

gentle reminder - is this OK to land now? Thanks!

[RKSimon]

llvm/test/lit.cfg.py

87

Do we still need this?

[mtrofin]

llvm/test/lit.cfg.py

87

Not for this change. It may help with other changes where we want to both introduce the desirable behavior for whole directories, and piecemeal opt out tests. But I can remove it from here and reintroduce it when we hit the problem - wdyt?

(for completeness, as previously discussed, once all tests are clean, we then remove the local.lit.cfg changes, and would replace any %FileCheckWithUnusedPrefixes% with the expanded FileCheck --allow-unused-prefixes)

[RKSimon]

LGTM, cheers - please drop the %FileCheckWithUnusedPrefixes% change until/if we actually need it.

[mtrofin]

Scoped changes to just llvm/test/Analysis

llvm/test/Analysis/lit.local.cfg

+# -*- Python -*- vim: set ft=python ts=4 sw=4 expandtab tw=79:
+from lit.llvm.subst import ToolSubst
+
+fc = ToolSubst('FileCheck', unresolved='fatal')
+# insert this first. Then, we'll first update the blank FileCheck command; then,
+# the default substitution of FileCheck will replace it to its full path.
+config.substitutions.insert(0, (fc.regex, 
+    'FileCheck --allow-unused-prefixes=false'))

llvm/test/lit.cfg.py

def get_asan_rtlib():
     host_lib_dir = os.path.join(os.path.dirname(config.host_cc), '../lib')
     asan_dylib_dir_pattern = host_lib_dir + \
         '/clang/*/lib/darwin/libclang_rt.asan_osx_dynamic.dylib'
     found_dylibs = glob.glob(asan_dylib_dir_pattern)
     if len(found_dylibs) != 1:
         return ''
     return found_dylibs[0]
 
+# Define this first. Afterwards, use_default_substitutions will add the rule for
+# expanding FileCheck to the full path.
+config.substitutions.append(('%FileCheckWithUnusedPrefixes%', 
+    'FileCheck --allow-unused-prefixes=true'))
 
 llvm_config.use_default_substitutions()
 
 # Add site-specific substitutions.
 config.substitutions.append(('%llvmshlibdir', config.llvm_shlib_dir))
 config.substitutions.append(('%shlibext', config.llvm_shlib_ext))
 config.substitutions.append(('%exeext', config.llvm_exe_ext))
 
-
 lli_args = []
 # The target triple used by default by lli is the process target triple (some
 # triple appropriate for generating code for the current process) but because
 # we don't support COFF in MCJIT well enough for the tests, force ELF format on
 # Windows.  FIXME: the process target triple should be used here, but this is
 # difficult to obtain on Windows.
 if re.search(r'cygwin|windows-gnu|windows-msvc', config.host_triple):
     lli_args = ['-mtriple=' + config.host_triple + '-elf']