[PATCH, RFC] Fix PR16454

Chandler Carruth chandlerc at google.com
Wed Jun 26 14:15:09 PDT 2013


On Wed, Jun 26, 2013 at 1:58 PM, Bill Schmidt
<wschmidt at linux.vnet.ibm.com>wrote:

> http://llvm.org/bugs/show_bug.cgi?id=16454 demonstrates that altivec.h
> is being included along paths where it doesn't make much sense to do so.
> This file is auto-included when compiling with -faltivec.  What I'm
> proposing is to avoid copying the -faltivec flag when the action being
> taken is to invoke the assembler or the preprocessor.
>
> I've verified that this fixes the test case in question without
> introducing any unit-test or test-suite regressions.  I also verified
> that preprocessing a file requiring altivec extensions (like "vector
> float x;") with the revised Clang and then compiling the resulting
> preprocessed file still works correctly.
>
> However, I really don't know anything about how all the driver machinery
> works, so I'd appreciate some extra eyes on this before I commit.  I'm
> CC'ing a few folks that were suggested.
>
> For what it's worth, the test case in question is fixed by the
> isa<PreprocessJobAction> test.  The other test is there just because it
> makes sense to me.
>
> I will add the original test from the PR as a unit test if this approach
> passes muster.
>

This is fine. Also add tests for the other cases you highlight. You can do
this very easily as driver tests that just check for the existence or
non-existence of -faltivec in the CC1 invocation, and uses '-x ...' and
other flags to control the actions selected.

Some comments on the patch:

-  Args.AddLastArg(CmdArgs, options::OPT_faltivec);
> +  // -faltivec causes inclusion of altivec.h, which makes no sense for
> +  // an assembly or preprocess action.
>

I would say instead that the AltiVec language extensions aren't relevant
for assembling or processing. It's not about the flag, it's about the
extensions being C language extensions.

I wonder if it would be useful to also add an error to lib/Frontend when
-faltivec is passed for these weird job types... Maybe just an assert...


> +  if (!isa<AssembleJobAction>(JA) && !isa<PreprocessJobAction>(JA))
> +    Args.AddLastArg(CmdArgs, options::OPT_faltivec);
>    Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_show_template_tree);
>    Args.AddLastArg(CmdArgs, options::OPT_fno_elide_type);
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130626/c1326505/attachment.html>


More information about the cfe-commits mailing list