This is an archive of the discontinued LLVM Phabricator instance.

[sanitizers] Make it possible to XFAIL on the effective target, not just the default.
ClosedPublic

Authored by dsanders on Jul 26 2016, 5:09 AM.

Details

Summary

The triple is not the right thing to XFAIL on since LIT only sees the default
triple and not the effective triple chosen by any -target option in the RUN
directives. This discrepancy is shown in the table below:

Default Triple   | Options                           | XFAIL  | LIT's expected result | Desired expectation
=================+===================================+========+=======================+====================
mips-linux-gnu   | -target mips-linux-gnu            |        | Pass                  | Pass  
mips-linux-gnu   | -target mips64-linux-gnu -mabi=64 |        | Pass                  | Pass  
mips-linux-gnu   | -target mips-linux-gnu            | mips   | Fail                  | Fail  
mips-linux-gnu   | -target mips64-linux-gnu -mabi=64 | mips   | Fail                  | Fail/Pass* (debatable**)
mips-linux-gnu   | -target mips-linux-gnu            | mips-  | Fail                  | Fail  
mips-linux-gnu   | -target mips64-linux-gnu -mabi=64 | mips-  | Fail                  | Pass* 
mips-linux-gnu   | -target mips-linux-gnu            | mips64 | Pass                  | Pass  
mips-linux-gnu   | -target mips64-linux-gnu -mabi=64 | mips64 | Pass                  | Fail* 
mips64-linux-gnu | -target mips-linux-gnu            |        | Pass                  | Pass  
mips64-linux-gnu | -target mips64-linux-gnu -mabi=64 |        | Pass                  | Pass  
mips64-linux-gnu | -target mips-linux-gnu            | mips   | Fail                  | Fail* 
mips64-linux-gnu | -target mips64-linux-gnu -mabi=64 | mips   | Fail                  | Fail/Pass (debatable**)
mips64-linux-gnu | -target mips-linux-gnu            | mips-  | Pass                  | Fail* 
mips64-linux-gnu | -target mips64-linux-gnu -mabi=64 | mips-  | Pass                  | Pass  
mips64-linux-gnu | -target mips-linux-gnu            | mips64 | Fail                  | Pass* 
mips64-linux-gnu | -target mips64-linux-gnu -mabi=64 | mips64 | Fail                  | Fail  
x64_64-linux-gnu | -target i386-linux-gnu            |        | Pass                  | Pass
x64_64-linux-gnu | -target x86_64-linux-gnu          |        | Pass                  | Pass
x64_64-linux-gnu | -target i386-linux-gnu            | i386   | Pass                  | Fail*
x64_64-linux-gnu | -target x86_64-linux-gnu          | i386   | Pass                  | Pass
x64_64-linux-gnu | -target i386-linux-gnu            | x86_64 | Fail                  | Pass
x64_64-linux-gnu | -target x86_64-linux-gnu          | x86_64 | Fail                  | Fail*
* These all differ from LIT's current behaviour.
** People's expectations vary depending on whether they know that LIT does a
 substring match on the default triple or think it's an exact match on an
 architecture.

This patch adds "target-is-${target_arch}" to the available features list and
updates the mips XFAIL's to use them. XFAIL'ing on these features will
correctly account for the target being tested. Making the table:

Options                           | XFAIL            | LIT's expected result
==================================+==================+======================
-target mips-linux-gnu            |                  | Pass
-target mips64-linux-gnu -mabi=64 |                  | Pass
-target mips-linux-gnu            | target-is-mips   | Fail
-target mips64-linux-gnu -mabi=64 | target-is-mips   | Pass
-target mips-linux-gnu            | target-is-mips64 | Pass
-target mips64-linux-gnu -mabi=64 | target-is-mips64 | Fail
-target i386-linux-gnu            |                  | Pass
-target x86_64-linux-gnu          |                  | Pass
-target i386-linux-gnu            | target-is-i386   | Fail
-target x86_64-linux-gnu          | target-is-i386   | Pass
-target i386-linux-gnu            | target-is-x86_64 | Pass
-target x86_64-linux-gnu          | target-is-x86_64 | Fail

Diff Detail

Event Timeline

dsanders updated this revision to Diff 65498.Jul 26 2016, 5:09 AM
dsanders retitled this revision from to [sanitizers] Make it possible to XFAIL on the effective target, not just the default..
dsanders updated this object.
dsanders added subscribers: samsonov, llvm-commits.
dsanders updated this object.Jul 26 2016, 5:12 AM
dsanders updated this object.

No documentation of the target-is-xxx feature. This really matters because the difference between 'mips64' and 'target-is-mips64' seems to be very subtle and even somewhat magical.

dsanders updated this revision to Diff 67141.Aug 8 2016, 3:16 AM

Added documentation of target-is-${arch} and the quirks that make it necessary.
There's no place for such documentation yet so I've created a new file to go
along with TestingGuide.rst from the llvm project.

Also, there's broken cross reference to the llvm document at the moment.
Phabricator won't let me add the necessary anchor to llvm/docs/TestingGuide.rst
in this review but I'll add it in the commit.

probinson accepted this revision.Aug 8 2016, 7:26 PM
probinson added a reviewer: probinson.

That is a really clear description, thanks! LGTM.

This revision is now accepted and ready to land.Aug 8 2016, 7:26 PM
dsanders closed this revision.Aug 9 2016, 4:58 AM
This revision was automatically updated to reflect the committed changes.