Page MenuHomePhabricator

Remove lit feature object-emission
ClosedPublic

Authored by ted on Jan 28 2020, 11:21 AM.

Details

Summary

The lit feature object-emission was added because Hexagon did not support the integrated assembler, so some tests needed to be turned off with a Hexagon target. Hexagon now supports the integrated assembler, so this feature can be removed.

Diff Detail

Event Timeline

ted created this revision.Jan 28 2020, 11:21 AM
ted added a comment.Feb 5 2020, 1:09 PM

Friendly ping

JDevlieghere accepted this revision.Feb 5 2020, 4:18 PM

This seems pretty straightforward, LGTM.

This revision is now accepted and ready to land.Feb 5 2020, 4:18 PM
ted updated this revision to Diff 243656.Feb 10 2020, 12:42 PM

File llvm/test/DebugInfo/X86/vla.ll was modified so the diff did not apply anymore. object-emission was removed from it, so I'm removing it from this diff.

This revision was automatically updated to reflect the committed changes.
vsk added a subscriber: vsk.Tue, Mar 17, 4:38 PM

@ted @JDevlieghere I think this caused a bunch of tests to be disabled, because "x86" is not an available feature.

$ ./bin/llvm-lit /Users/vsk/src/llvm-project-master/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll 
-- Testing: 1 tests, 1 workers --
UNSUPPORTED: LLVM :: DebugInfo/X86/dwarf-callsite-related-attrs.ll (1 of 1)

$ ./bin/llvm-lit /Users/vsk/src/llvm-project-master/llvm/test/DebugInfo/X86/arguments.ll                   
-- Testing: 1 tests, 1 workers --
UNSUPPORTED: LLVM :: DebugInfo/X86/arguments.ll (1 of 1)

If 'object-emission' is gone, can we fix this by removing all instances of 'REQUIRES: x86'?

In D73568#1927929, @vsk wrote:

If 'object-emission' is gone, can we fix this by removing all instances of 'REQUIRES: x86'?

Interesting. I'd have thought this would match an x86_64-foo-foo target triple, but perhaps not.
In any case, tests under an X86 directory should not need an explicit REQUIRES because the lit.local.cfg takes care of that.

vsk added a comment.Thu, Mar 19, 8:50 AM
In D73568#1927929, @vsk wrote:

If 'object-emission' is gone, can we fix this by removing all instances of 'REQUIRES: x86'?

Interesting. I'd have thought this would match an x86_64-foo-foo target triple, but perhaps not.
In any case, tests under an X86 directory should not need an explicit REQUIRES because the lit.local.cfg takes care of that.

Yeah, I think the available features checks require an exact match (not sure if wildcards are supported, but in any case there was no wildcard used here).

I'll look into re-enabling the tests now. Removing the 'REQUIRES' line is simple enough but as the tests were disabled for a while, several of them are broken.

vsk added a comment.Thu, Mar 19, 9:30 AM

My earlier comment was wrong, none of the tests broke, I just had a stale build directory. I've re-enabled the x86 tests in 5e6e545cbab05589c8fcef5f03eca60dc227a6ca.