llvm dev - Oct 2016 - How is target_triple/default_triple handled in tests?

As part of trying to fix PR30610 (ThinLTO with module inline asm), I wanted
to add an assert that we have a Target and an MCAsmParser when we have
non-null module inline asm in IRObjectFile::CollectAsmUndefinedRefs.
Otherwise it silently fails to parse the module inline asm, which I
initially found when trying to add an 'opt' based test for my fix
(because
'opt' wasn't initializing asm parsers, something I am fixing).

However, I had a test case failure in test/LTO/X86/current-section.ll,
which has module asm, but no target triple. However, it does have a
"REQUIRES: default_triple". Mehdi, looks like you added this as part
of
D12660. I found that this is set to true in test/lit.cfg, and when I print
out the config.target_triple there it is x86_64-unknown-linux-gnu. But when
I print out the target triple on the Module in CollectAsmUndefinedRefs it
is null. How is the config.target_triple supposed to propagate into the
test - I assume that is the intention of requiring a default_triple?

Thanks,
Teresa

-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161006/be683fe4/attachment.html>

> On Oct 6, 2016, at 1:40 PM, Teresa Johnson via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> As part of trying to fix PR30610 (ThinLTO with module inline asm), I wanted
to add an assert that we have a Target and an MCAsmParser when we have non-null
module inline asm in IRObjectFile::CollectAsmUndefinedRefs. Otherwise it
silently fails to parse the module inline asm, which I initially found when
trying to add an 'opt' based test for my fix (because 'opt'
wasn't initializing asm parsers, something I am fixing).
> 
> However, I had a test case failure in test/LTO/X86/current-section.ll,
which has module asm, but no target triple. However, it does have a
"REQUIRES: default_triple". Mehdi, looks like you added this as part
of D12660.
> I found that this is set to true in test/lit.cfg, and when I print out the
config.target_triple there it is x86_64-unknown-linux-gnu. But when I print out
the target triple on the Module in CollectAsmUndefinedRefs it is null. How is
the config.target_triple supposed to propagate into the test - I assume that is
the intention of requiring a default_triple?
Tools are usually adding the default triple when there is none. This is not a
“feature” I like though.
The “default” triple is defined at configuration time, either you supply the
cmake option otherwise it default to your host.
It is likely that llvm-lto is not overriding the triple (I rather have it
asserting that there is a triple by the way).

Adding a triple to any test that relies on inline module asm seem the right
thing to do to me, otherwise the test would break if someone were to configure
with a default triple for a different target.

— 
Mehdi

On Thu, Oct 6, 2016 at 1:48 PM, Mehdi Amini <mehdi.amini at apple.com>
wrote:
>
> > On Oct 6, 2016, at 1:40 PM, Teresa Johnson via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >
> > As part of trying to fix PR30610 (ThinLTO with module inline asm), I
> wanted to add an assert that we have a Target and an MCAsmParser when we
> have non-null module inline asm in IRObjectFile::CollectAsmUndefinedRefs.
> Otherwise it silently fails to parse the module inline asm, which I
> initially found when trying to add an 'opt' based test for my fix
(because
> 'opt' wasn't initializing asm parsers, something I am fixing).
> >
> > However, I had a test case failure in test/LTO/X86/current-section.ll,
> which has module asm, but no target triple. However, it does have a
> "REQUIRES: default_triple". Mehdi, looks like you added this as
part of
> D12660.
> > I found that this is set to true in test/lit.cfg, and when I print out
> the config.target_triple there it is x86_64-unknown-linux-gnu. But when I
> print out the target triple on the Module in CollectAsmUndefinedRefs it is
> null. How is the config.target_triple supposed to propagate into the test -
> I assume that is the intention of requiring a default_triple?
>
> Tools are usually adding the default triple when there is none. This is
> not a “feature” I like though.
> The “default” triple is defined at configuration time, either you supply
> the cmake option otherwise it default to your host.
> It is likely that llvm-lto is not overriding the triple (I rather have it
> asserting that there is a triple by the way).
>
I see that 'opt' and 'llc' have an -mtriple option, is that what
you meant?
>
> Adding a triple to any test that relies on inline module asm seem the
> right thing to do to me, otherwise the test would break if someone were to
> configure with a default triple for a different target.
>
I'll go ahead and add a target triple to that test then.

Thanks!
Teresa

>
> —
> Mehdi
>
>

-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161006/d80d3b02/attachment.html>