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

Pengfei Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 10 21:24:37 PST 2020


pengfei 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,))
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93078



More information about the llvm-commits mailing list