[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin marked 2 inline comments as done.
mtrofin added inline comments.


================
Comment at: llvm/utils/UpdateTestChecks/common.py:296
           else:
-            if prefix == prefixes[-1]:
-              warn('Found conflicting asm under the same prefix: %r!' % (prefix,))
----------------
pengfei wrote:
> mtrofin wrote:
> > pengfei wrote:
> > > I didn't dig into it too much, but the code looks to me that if all prefixes for the func don't meet the condition (already iterated to the last one), it reports warnings to user.
> > > It seems you just removed the warnings instead of fixing the problem. So users hardly be aware of there's real conflict on their prefixes lists on RUN command line.
> > I'm not sure what the original intent was there, but I'm not sure it serves a real reason anymore. See for example the "common-label-different-bodies-2.ll" test case, which would fail today (i.e. it'd produce the wrong outcome, and warn) - can't see why that would be desirable. Maybe I'm missing something?
> I did an experiment: if you change both prefixes of the RUNs in your tests to the same one, you will find it doesn't generate checks for them and doesn't report any warning. I guess that's should be the original intent here.
Ah, thanks! Like so (picking on the -2 file):

; RUN: llc < %s -mtriple=i686-unknown-linux-gnu -mattr=+sse2 | FileCheck %s --check-prefixes=A
; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefixes=A

Got it. Indeed, the proposed behavior doesn't warn and doesn't generate an output for A; and the old behavior was generating incorrect asserts and outputing a warning.

This seems to end up as an unused prefixes case - (or am I holding a hammer and everything is a nail?). I suppose we can handle it in the tool after all the RUN lines are processed, but there are a few users of build_function_body_dictionary, so it'd be best to wrap the func_dict population; or, we can let it silently fail, and then FileCheck will complain quickly thereafter about duplicate prefixes.

Happy to do the wrapper in the tools, especially if this is something that happens frequently?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93078