[PATCH, RFC] Fix PR16454

[Hal Finkel]

----- Original Message -----
> 
> 
> On Wed, 2013-06-26 at 16:14 -0500, Hal Finkel wrote:
> > ----- Original Message -----
> > > 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.
> > 
> > I think that this makes sense; so the job action is not available
> > from within Frontend/CompilerInvocation.cpp where the file is
> > actually included?
> 
> Right, at least so far as I can tell.  That code doesn't appear to be
> aware whence it was called.

Looking at this, it seems that it is.

ParsePreprocessorArgs is currently called from CompilerInvocation::CreateFromArgs, and that function passes Res.getPreprocessorOpts() to ParsePreprocessorArgs. Just prior to that, there is code that uses Res.getFrontendOpts().ProgramAction, and also has a 'InputKind DashX' variable. Look around CompilerInvocation.cpp lines 1590-1622.

 -Hal

> 
> > 
> > To confirm, if you compile something with clang that crashes, thus
> > generating a preprocessed input file, the contents of altivec.h
> > will no longer be copied into that file, correct?
> 
> Well, this is interesting.  I get different behavior with and without
> --save-temps:
> 
> $ cat crashit.c
> vector float zowie wheresmycomma;
> 
> $ $LLVM_INSTALL/bin/clang -E -maltivec crashit.c
> # 1 "crashit.c"
> # 1 "<built-in>" 1
> # 1 "<built-in>" 3
> # 155 "<built-in>" 3
> # 1 "<command line>" 1
> # 1 "<built-in>" 2
> # 1 "crashit.c" 2
> vector float zowie wheresmycomma;
> 
> $ $LLVM_INSTALL/bin/clang -c -maltivec crashit.c
> crashit.c:1:19: error: expected ';' after top level declarator
> vector float zowie wheresmycomma;
>                   ^
>                   ;
> 1 error generated.
> 
> $ $LLVM_INSTALL/bin/clang -c -maltivec --save-temps crashit.c
> In file included from <built-in>:158:
> <command line>:1:10: fatal error: 'altivec.h' file not found
> #include "altivec.h"
>          ^
> 1 error generated.
> 
> 
> I guess this is why I need a driver expert.  Can anyone explain what
> code path we're going down with --save-temps that causes the
> different
> behavior?  (BTW, --save-temps produces a crashit.i identical to the
> output from the -E command.)
> 
> Thanks, Hal!
> 
> Bill
> 
> > 
> >  -Hal
> > 
> > > 
> > > Thanks,
> > > Bill
> > > 
> > > 
> > > Index: lib/Driver/Tools.cpp
> > > ===================================================================
> > > --- lib/Driver/Tools.cpp	(revision 184947)
> > > +++ lib/Driver/Tools.cpp	(working copy)
> > > @@ -2844,7 +2844,10 @@ void Clang::ConstructJob(Compilation &C,
> > > const
> > > Job
> > >    Args.AddLastArg(CmdArgs, options::OPT_flimit_debug_info);
> > >    Args.AddLastArg(CmdArgs, options::OPT_fno_limit_debug_info);
> > >    Args.AddLastArg(CmdArgs, options::OPT_fno_operator_names);
> > > -  Args.AddLastArg(CmdArgs, options::OPT_faltivec);
> > > +  // -faltivec causes inclusion of altivec.h, which makes no
> > > sense
> > > for
> > > +  // an assembly or preprocess action.
> > > +  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);
> > >  
> > > 
> > > 
> > > _______________________________________________
> > > cfe-commits mailing list
> > > cfe-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > > 
> > 
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory