Make changes to clang driver so it can pass -target-abi option to created jobs.
Details
Diff Detail
Event Timeline
It needs a testcase and I don't see where Opts.ABI is used.
By the way, please don't set custom access policies. There's no need for them.
tools/driver/cc1as_main.cpp | ||
---|---|---|
82 ↗ | (On Diff #15711) | Not initialized in constructor. Also, can you point me at the code that uses this variable? I can only see the code that sets it. |
Not sure that the getTargetFeatures routine is a good place to configure the -target-abi option.
lib/Driver/Tools.cpp | ||
---|---|---|
1793 | The getTargetFeatures routine called from the ClangAs::ConstructJob and from the Clang::ConstructJob. In the second case we add the -target-abi option twice. The first time when we call the getTargetFeatures and the second time when we call Clang::AddMIPSTargetArgs. I do not think it is a good idea. |
I was suspicious about this myself, but couldn't find the proper place to put the code in. Do you please have any idea where it would suite better?
Maybe create new method ClangAs::AddMIPSTargetArgs with -target-abi handling and call this method from ClangAs::ConstructJob. I think that code duplication is a minor problem here.
A new method ClangAs::AddMIPSTargetArgs with -target-abi handling has been created. This method is called from ClangAs::ConstructJob to handle this particular task.
tools/driver/cc1as_main.cpp | ||
---|---|---|
192 ↗ | (On Diff #16140) | Where do we use the value stored in the Opts.ABI? |
tools/driver/cc1as_main.cpp | ||
---|---|---|
192 ↗ | (On Diff #16140) | I I understood correctly these options are passed to assembler when it is invoked with -cc1 option. Please correct me if I'm wrong. |
And the patch still needs a test case.
tools/driver/cc1as_main.cpp | ||
---|---|---|
192 ↗ | (On Diff #16140) | At the line cc1as_main.cpp:82 you add new field ABI to the AssemblerInvocation structure. At the line cc1as_main.cpp:191 you initialize this filed by the -target-abi arguments. But where is this new field used? For example the AssemblerInvocation::CPU filed is initialized at the line 186 and passed to the createMCSubtargetInfo at the line 357. |
Removed ABI field from AssemblerInvocation class. Added test cases for integrated assembler launch.
You'll want to remove the -target-feature bits of this code... honestly it shouldn't even compile at this point.
Actually this patch to the testcase doesn't appear to be against top of tree - can you rebase against that?
Thanks.
-eric
Patch is rebased to the TOT. Passing -target-feature o32,n32,n64 options is disabled to remove warnings.
The getTargetFeatures routine called from the ClangAs::ConstructJob and from the Clang::ConstructJob. In the second case we add the -target-abi option twice. The first time when we call the getTargetFeatures and the second time when we call Clang::AddMIPSTargetArgs. I do not think it is a good idea.