[PATCH] D77780: Remove -implicit-check-not=foo from X86/disassemble-functions.test

Thu Apr 9 01:36:12 PDT 2020

jhenderson requested changes to this revision.
jhenderson added a comment.
This revision now requires changes to proceed.

In D77780#1971300 <https://reviews.llvm.org/D77780#1971300>, @MaskRay wrote:

> Can you double check the non-determinism after D77640 <https://reviews.llvm.org/D77640>?

As @MaskRay mentions, I just fixed some non-determinism in how llvm-objdump disassembles things when there are two sections with the same address. This should fix the occasional failures you have been seeing. I also have a follow-up change in progress (not quite ready for review yet, but likely either today or Tuesday when I'm working next), which will make symbols more likely to be found than they were before.

> 1, In fact this test-self is not good, (should not use -implicit-check*-not*='foo'). //objdump tool will show foo here.

This test should check that the foo function is NOT disassembled, because it was not requested by --disassemble-symbols. That is why the --implicit-check-not is there. Please do not delete it.

I also noticed that the --implicit-check-not pattern you posted here is not what is in the test now. See commit rG27f303924e0b32e22820fa38cb659e9694954784 <https://reviews.llvm.org/rG27f303924e0b32e22820fa38cb659e9694954784> which changed that and would also prevent this test failing I believe for you, even without the mentioned fix.

What might need doing is fixing the YAML input to not have two non-empty sections at the same address, as this isn't valid.

Aside: there are a few legacy bits in this and related tests referring to "--disassemble-functions" which are incorrect, since the switch was renamed. We should clean those up at some point.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77780