llvm dev - Sep 2012 - [LLVMdev] [llvm-commits] [llvm] r160610 - /llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp

Andy,

Yes, it was wrong, as long as RemoteTarget is available for the target.

If you would revert it, please consider then;

  - non-MCJIT stuff
  - Running different environment on the JIT
    e.g.) --build=--host=i686-linux --target=x86_64-linux
  - Host-incapable target on the JIT
    e.g.) --build=--host=x86_64-linux ---target=i686-mingw32

If JIT is unavailable for the default target, tests in
test/ExecutionEngine and unittests/ExecutionEngine/JIT should be
suppressed. My r160610 intended to tweak tests on cross targets (not
to suppress, but to appease).

I wonder if we should honor the triple in Module, though.

Feel free to update, unless breaking tests, thank you.

...Takumi


2012/9/12 Kaylor, Andrew <andrew.kaylor at
intel.com>:
> Hi,
>
> I realize that this is an old commit, but I just came across a problem
related to this change and was hoping to enquire as to the rationale here.
>
> It is possible to provide a target triple to lli via the '-mtriple'
command line option.  However, as a result of the change below, the user
specified triple gets ignored in favor of LLVM_HOSTTRIPLE.  This seems wrong.
>
> I recognize that problems can arise if you try to use a target other than
the default host triple with JIT or MCJIT.  However, with MCJIT it may be
desirable to provide an environment string via the triple to generate MachO or
ELF objects on platforms that wouldn't normally use those formats.  In
particular, I am working on adding support for generating ELF objects on Windows
(because COFF isn't supported by MCJIT).
>
> The change below prevents that.  As such, I would like to revert this
change, or at least modify this to pick-up the environment from the module's
target triple.
>
> Does that seem reasonable?
>
> -Andy
>
>
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at
cs.uiuc.edu] On Behalf Of NAKAMURA Takumi
> Sent: Saturday, July 21, 2012 8:05 PM
> To: llvm-commits at cs.uiuc.edu
> Subject: [llvm-commits] [llvm] r160610 -
/llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp
>
> Author: chapuni
> Date: Sat Jul 21 22:04:57 2012
> New Revision: 160610
>
> URL: http://llvm.org/viewvc/llvm-project?rev=160610&view=rev
> Log:
> ExecutionEngine/TargetSelect.cpp: Override default triple as
LLVM_HOSTTRIPLE.
>
> In current implementation, JIT should run only on host.
>
> Modified:
>     llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp
>
> Modified: llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp
> URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp?rev=160610&r1=160609&r2=160610&view=diff
>
=============================================================================>
--- llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp (original)
> +++ llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp Sat Jul 21 22:04:57
> +++ 2012
> @@ -26,7 +26,7 @@
>  using namespace llvm;
>
>  TargetMachine *EngineBuilder::selectTarget() {
> -  Triple TT(M->getTargetTriple());
> +  Triple TT(LLVM_HOSTTRIPLE);
>    return selectTarget(TT, MArch, MCPU, MAttrs);  }

Hi Takumi,

I'm a bit confused as to what is supposed to happen in the cross building
scenarios.  For instance, if host=x86_64-linux and target=i686-mingw32, what
should the MCJIT tests do?  Should they be suppressed because the architectures
don't match?  If so, what about the case where host=x86_64-linux and
target=x86_64-mingw32?

I'm modifying the MCJIT tests so that they will provide an explicit target
triple on the run line.  In the case where the host OS is Windows, I'm
appending "-elf" to the target triple.  I think that should be
sufficient to make things work in the cases where the host triple and the target
triple are compatible, and then I can suppress the cases where host and target
aren't compatible in the lit.local.cfg file.

Does that sound right?

My concern is that I don't quite know how to predict what will happen on
some of the buildbots aside from checking something in and seeing what happens. 
Hopefully any obvious problems will be caught in code review.

Aside from the testing, it seems to me that TargetSelect should always use the
Module target triple, and if it isn't compatible with the host environment
then it will just fail.

-Andy

-----Original Message-----
From: NAKAMURA Takumi [mailto:geek4civic at gmail.com] 
Sent: Tuesday, September 11, 2012 5:13 PM
To: Kaylor, Andrew
Cc: llvm-commits at cs.uiuc.edu; llvmdev
Subject: Re: [llvm-commits] [llvm] r160610 -
/llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp

Andy,

Yes, it was wrong, as long as RemoteTarget is available for the target.

If you would revert it, please consider then;

  - non-MCJIT stuff
  - Running different environment on the JIT
    e.g.) --build=--host=i686-linux --target=x86_64-linux
  - Host-incapable target on the JIT
    e.g.) --build=--host=x86_64-linux ---target=i686-mingw32

If JIT is unavailable for the default target, tests in test/ExecutionEngine and
unittests/ExecutionEngine/JIT should be suppressed. My r160610 intended to tweak
tests on cross targets (not to suppress, but to appease).

I wonder if we should honor the triple in Module, though.

Feel free to update, unless breaking tests, thank you.

...Takumi


2012/9/12 Kaylor, Andrew <andrew.kaylor at
intel.com>:
> Hi,
>
> I realize that this is an old commit, but I just came across a problem
related to this change and was hoping to enquire as to the rationale here.
>
> It is possible to provide a target triple to lli via the '-mtriple'
command line option.  However, as a result of the change below, the user
specified triple gets ignored in favor of LLVM_HOSTTRIPLE.  This seems wrong.
>
> I recognize that problems can arise if you try to use a target other than
the default host triple with JIT or MCJIT.  However, with MCJIT it may be
desirable to provide an environment string via the triple to generate MachO or
ELF objects on platforms that wouldn't normally use those formats.  In
particular, I am working on adding support for generating ELF objects on Windows
(because COFF isn't supported by MCJIT).
>
> The change below prevents that.  As such, I would like to revert this
change, or at least modify this to pick-up the environment from the module's
target triple.
>
> Does that seem reasonable?
>
> -Andy
>
>
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu 
> [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of NAKAMURA Takumi
> Sent: Saturday, July 21, 2012 8:05 PM
> To: llvm-commits at cs.uiuc.edu
> Subject: [llvm-commits] [llvm] r160610 - 
> /llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp
>
> Author: chapuni
> Date: Sat Jul 21 22:04:57 2012
> New Revision: 160610
>
> URL: http://llvm.org/viewvc/llvm-project?rev=160610&view=rev
> Log:
> ExecutionEngine/TargetSelect.cpp: Override default triple as
LLVM_HOSTTRIPLE.
>
> In current implementation, JIT should run only on host.
>
> Modified:
>     llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp
>
> Modified: llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Tar
> getSelect.cpp?rev=160610&r1=160609&r2=160610&view=diff
> =====================================================================>
=======> --- llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp (original)
> +++ llvm/trunk/lib/ExecutionEngine/TargetSelect.cpp Sat Jul 21 
> +++ 22:04:57
> +++ 2012
> @@ -26,7 +26,7 @@
>  using namespace llvm;
>
>  TargetMachine *EngineBuilder::selectTarget() {
> -  Triple TT(M->getTargetTriple());
> +  Triple TT(LLVM_HOSTTRIPLE);
>    return selectTarget(TT, MArch, MCPU, MAttrs);  }

2012/9/13 Kaylor, Andrew <andrew.kaylor at
intel.com>:
> I'm a bit confused as to what is supposed to happen in the cross
building scenarios.  For instance, if host=x86_64-linux and target=i686-mingw32,
what should the MCJIT tests do?  Should they be suppressed because the
architectures don't match?  If so, what about the case where
host=x86_64-linux and target=x86_64-mingw32?
Basically, I don't think local JIT could execute other environments.
Both {i686|x86_64}-mingw32 are incompatible to Linux, at least.

It should be suppressed unless the target could run as RemoteTarget.

BTW, do you think -target i686-linux should run under x86_64 binary?
(I wish so, but it would involve some issues...)
> I'm modifying the MCJIT tests so that they will provide an explicit
target triple on the run line.  In the case where the host OS is Windows,
I'm appending "-elf" to the target triple.  I think that should be
sufficient to make things work in the cases where the host triple and the target
triple are compatible, and then I can suppress the cases where host and target
aren't compatible in the lit.local.cfg file.
>
> Does that sound right?
Do you mean; "-mtriple=%{targettriple_provided_by_lit}-elf" ?
I think MCJIT may reinterpret the triple to be accepted by elf.
> My concern is that I don't quite know how to predict what will happen
on some of the buildbots aside from checking something in and seeing what
happens.  Hopefully any obvious problems will be caught in code review.
I can investigate failures if they happened in my builders. All of my
builders are in my room :)
> Aside from the testing, it seems to me that TargetSelect should always use
the Module target triple, and if it isn't compatible with the host
environment then it will just fail.
It should be better that Target may be overridden. It was the reason
why I tweaked TargetSelect().

...Takumi