Re: [clang] 7342912 - [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (3)

2019-11-20 Thread Puyan Lotfi via cfe-commits
I have a solution, stay tuned.

So it turns out since the clang driver has special behavior on Darwin for 
handling Universal apps, there is a different code path that happens during 
BuildActions. clang::driver::Driver::BuildUniversalActions is called instead of 
BuildActions on Darwin, and what BuildUniversalActions ends up doing is it 
calls BuildActions and takes the driver actions returned and wraps them all 
into a BindArchAction each. So when I check for a IfsMergeAction in the driver 
instead I am getting a BindArchAction that has an IfsMergeAction inside of it.

I will post a fix to get Green Dragon and the Fuchsia Darwin bots green again, 
and also post a post-commit review on Phab for good measure. 

Thanks Again for the heads up Alex.

Puyan

Sent with ProtonMail Secure Email.

‐‐‐ Original Message ‐‐‐
On Wednesday, November 20, 2019 4:52 PM, Puyan Lotfi  wrote:

> Looking into this. Thanks for the heads up. 
> 

> PL
> 

> Sent with ProtonMail Secure Email.
> 

> ‐‐‐ Original Message ‐‐‐
> On Wednesday, November 20, 2019 4:12 PM, Alex L  wrote:
> 

> > Hi Puyan,
> > 

> > This commit caused two Clang failures on Darwin:
> > 

> >     Clang :: InterfaceStubs/merge-conflict-test.c
> >     Clang :: InterfaceStubs/object-float.c
> > 

> > Here's the build log from out bot:
> > http://lab.llvm.org:8080/green/job/clang-stage1-RA/3929/console
> > 

> > Can you please resolve the issue with the tests?
> > Thanks,
> > Alex
> > 

> > On Wed, 20 Nov 2019 at 14:16, Puyan Lotfi via cfe-commits 
> >  wrote:
> > 

> > > Author: Puyan Lotfi
> > > Date: 2019-11-20T16:22:50-05:00
> > > New Revision: 73429126c91c2065c6f6ef29b3eec1b7798502bb
> > > 

> > > URL: 
> > > https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb
> > > DIFF: 
> > > https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb.diff
> > > 

> > > LOG: [clang][IFS] Driver Pipeline: generate stubs after standard pipeline 
> > > (3)
> > > 

> > > Third Landing Attempt (dropping any linker invocation from clang driver):
> > > 

> > > Up until now, clang interface stubs has replaced the standard
> > > PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in
> > > conjunction with it. So what when you build your code you will get an
> > > a.out or lib.so as well as an interface stub file.
> > > 

> > > Example:
> > > 

> > > clang -shared -o libfoo.so -emit-interface-stubs ...
> > > 

> > > will generate both a libfoo.so and a libfoo.ifso. The .so file will
> > > contain the code from the standard compilation pipeline and the .ifso
> > > file will contain the ELF stub library.
> > > 

> > > Note: For driver-test.c I've added -S in order to prevent any bot 
> > > failures on
> > > bots that don't have the proper linker for their native triple. You could 
> > > always
> > > specify a triple like x86_64-unknown-linux-gnu and on bots like 
> > > x86_64-scei-ps4
> > > the clang driver would invoke regular ld instead of getting the error
> > > 'Executable "orbis-ld" doesn't exist!' but on bots like ppc64be and s390x 
> > > you'd
> > > get an error "/usr/bin/ld: unrecognised emulation mode: elf_x86_64"
> > > 

> > > Differential Revision: https://reviews.llvm.org/D70274
> > > 

> > > Added:
> > >     clang/test/InterfaceStubs/driver-test2.c
> > >     clang/test/InterfaceStubs/ppc.cpp
> > > 

> > > Modified:
> > >     clang/lib/Driver/Driver.cpp
> > >     clang/lib/Driver/ToolChains/InterfaceStubs.cpp
> > >     clang/lib/Driver/Types.cpp
> > >     clang/test/InterfaceStubs/driver-test.c
> > >     clang/test/InterfaceStubs/windows.cpp
> > > 

> > > Removed:
> > > 

> > > 
> > > diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
> > > index cdf4a579f431..83b5db3b2530 100644
> > > --- a/clang/lib/Driver/Driver.cpp
> > > +++ b/clang/lib/Driver/Driver.cpp
> > > @@ -292,10 +292,6 @@ phases::ID Driver::getFinalPhase(const 
> > > DerivedArgList &DAL,
> > >               (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
> > >      FinalPhase = phases::Compile;
> > > 

> > > -  // clang interface stubs
> > > -  } else if ((PhaseArg = 
> > > DAL.getLastArg(options::OPT_emit_interface_stubs))) {
> > > -    FinalPhase = phases::IfsMerge;
> > > -
> > >    // -S only runs up to the backend.
> > >    } else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
> > >      FinalPhase = phases::Backend;
> > > @@ -3502,6 +3498,68 @@ void Driver::BuildActions(Compilation &C, 
> > > DerivedArgList &Args,
> > >      Actions.push_back(
> > >          C.MakeAction(MergerInputs, types::TY_Image));
> > > 

> > > +  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
> > > +    llvm::SmallVector PhaseList;
> > > +    if (Args.hasArg(options::OPT_c)) {
> > > +      llvm::SmallVector 
> > > CompilePhaseList;
> > > +      types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList)

Re: [clang] 7342912 - [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (3)

2019-11-20 Thread Puyan Lotfi via cfe-commits
Looking into this. Thanks for the heads up. 

PL

Sent with ProtonMail Secure Email.

‐‐‐ Original Message ‐‐‐
On Wednesday, November 20, 2019 4:12 PM, Alex L  wrote:

> Hi Puyan,
> 

> This commit caused two Clang failures on Darwin:
> 

>     Clang :: InterfaceStubs/merge-conflict-test.c
>     Clang :: InterfaceStubs/object-float.c
> 

> Here's the build log from out bot:
> http://lab.llvm.org:8080/green/job/clang-stage1-RA/3929/console
> 

> Can you please resolve the issue with the tests?
> Thanks,
> Alex
> 

> On Wed, 20 Nov 2019 at 14:16, Puyan Lotfi via cfe-commits 
>  wrote:
> 

> > Author: Puyan Lotfi
> > Date: 2019-11-20T16:22:50-05:00
> > New Revision: 73429126c91c2065c6f6ef29b3eec1b7798502bb
> > 

> > URL: 
> > https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb
> > DIFF: 
> > https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb.diff
> > 

> > LOG: [clang][IFS] Driver Pipeline: generate stubs after standard pipeline 
> > (3)
> > 

> > Third Landing Attempt (dropping any linker invocation from clang driver):
> > 

> > Up until now, clang interface stubs has replaced the standard
> > PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in
> > conjunction with it. So what when you build your code you will get an
> > a.out or lib.so as well as an interface stub file.
> > 

> > Example:
> > 

> > clang -shared -o libfoo.so -emit-interface-stubs ...
> > 

> > will generate both a libfoo.so and a libfoo.ifso. The .so file will
> > contain the code from the standard compilation pipeline and the .ifso
> > file will contain the ELF stub library.
> > 

> > Note: For driver-test.c I've added -S in order to prevent any bot failures 
> > on
> > bots that don't have the proper linker for their native triple. You could 
> > always
> > specify a triple like x86_64-unknown-linux-gnu and on bots like 
> > x86_64-scei-ps4
> > the clang driver would invoke regular ld instead of getting the error
> > 'Executable "orbis-ld" doesn't exist!' but on bots like ppc64be and s390x 
> > you'd
> > get an error "/usr/bin/ld: unrecognised emulation mode: elf_x86_64"
> > 

> > Differential Revision: https://reviews.llvm.org/D70274
> > 

> > Added:
> >     clang/test/InterfaceStubs/driver-test2.c
> >     clang/test/InterfaceStubs/ppc.cpp
> > 

> > Modified:
> >     clang/lib/Driver/Driver.cpp
> >     clang/lib/Driver/ToolChains/InterfaceStubs.cpp
> >     clang/lib/Driver/Types.cpp
> >     clang/test/InterfaceStubs/driver-test.c
> >     clang/test/InterfaceStubs/windows.cpp
> > 

> > Removed:
> > 

> > 
> > diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
> > index cdf4a579f431..83b5db3b2530 100644
> > --- a/clang/lib/Driver/Driver.cpp
> > +++ b/clang/lib/Driver/Driver.cpp
> > @@ -292,10 +292,6 @@ phases::ID Driver::getFinalPhase(const DerivedArgList 
> > &DAL,
> >               (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
> >      FinalPhase = phases::Compile;
> > 

> > -  // clang interface stubs
> > -  } else if ((PhaseArg = 
> > DAL.getLastArg(options::OPT_emit_interface_stubs))) {
> > -    FinalPhase = phases::IfsMerge;
> > -
> >    // -S only runs up to the backend.
> >    } else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
> >      FinalPhase = phases::Backend;
> > @@ -3502,6 +3498,68 @@ void Driver::BuildActions(Compilation &C, 
> > DerivedArgList &Args,
> >      Actions.push_back(
> >          C.MakeAction(MergerInputs, types::TY_Image));
> > 

> > +  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
> > +    llvm::SmallVector PhaseList;
> > +    if (Args.hasArg(options::OPT_c)) {
> > +      llvm::SmallVector 
> > CompilePhaseList;
> > +      types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
> > +      llvm::copy_if(CompilePhaseList, std::back_inserter(PhaseList),
> > +                    [&](phases::ID Phase) { return Phase <= 
> > phases::Compile; });
> > +    } else {
> > +      types::getCompilationPhases(types::TY_IFS_CPP, PhaseList);
> > +    }
> > +
> > +    ActionList MergerInputs;
> > +
> > +    for (auto &I : Inputs) {
> > +      types::ID InputType = I.first;
> > +      const Arg *InputArg = I.second;
> > +
> > +      // Currently clang and the llvm assembler do not support generating 
> > symbol
> > +      // stubs from assembly, so we skip the input on asm files. For ifs 
> > files
> > +      // we rely on the normal pipeline setup in the pipeline setup code 
> > above.
> > +      if (InputType == types::TY_IFS || InputType == types::TY_PP_Asm ||
> > +          InputType == types::TY_Asm)
> > +        continue;
> > +
> > +      Action *Current = C.MakeAction(*InputArg, InputType);
> > +
> > +      for (auto Phase : PhaseList) {
> > +        switch (Phase) {
> > +        default:
> > +          llvm_unreachable(
> > +              "IFS Pipeline can only consist of Compile 

Re: [clang] 7342912 - [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (3)

2019-11-20 Thread Alex L via cfe-commits
Hi Puyan,

This commit caused two Clang failures on Darwin:

Clang :: InterfaceStubs/merge-conflict-test.c
Clang :: InterfaceStubs/object-float.c

Here's the build log from out bot:
http://lab.llvm.org:8080/green/job/clang-stage1-RA/3929/console

Can you please resolve the issue with the tests?
Thanks,
Alex

On Wed, 20 Nov 2019 at 14:16, Puyan Lotfi via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Puyan Lotfi
> Date: 2019-11-20T16:22:50-05:00
> New Revision: 73429126c91c2065c6f6ef29b3eec1b7798502bb
>
> URL:
> https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb
> DIFF:
> https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb.diff
>
> LOG: [clang][IFS] Driver Pipeline: generate stubs after standard pipeline
> (3)
>
> Third Landing Attempt (dropping any linker invocation from clang driver):
>
> Up until now, clang interface stubs has replaced the standard
> PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in
> conjunction with it. So what when you build your code you will get an
> a.out or lib.so as well as an interface stub file.
>
> Example:
>
> clang -shared -o libfoo.so -emit-interface-stubs ...
>
> will generate both a libfoo.so and a libfoo.ifso. The .so file will
> contain the code from the standard compilation pipeline and the .ifso
> file will contain the ELF stub library.
>
> Note: For driver-test.c I've added -S in order to prevent any bot failures
> on
> bots that don't have the proper linker for their native triple. You could
> always
> specify a triple like x86_64-unknown-linux-gnu and on bots like
> x86_64-scei-ps4
> the clang driver would invoke regular ld instead of getting the error
> 'Executable "orbis-ld" doesn't exist!' but on bots like ppc64be and s390x
> you'd
> get an error "/usr/bin/ld: unrecognised emulation mode: elf_x86_64"
>
> Differential Revision: https://reviews.llvm.org/D70274
>
> Added:
> clang/test/InterfaceStubs/driver-test2.c
> clang/test/InterfaceStubs/ppc.cpp
>
> Modified:
> clang/lib/Driver/Driver.cpp
> clang/lib/Driver/ToolChains/InterfaceStubs.cpp
> clang/lib/Driver/Types.cpp
> clang/test/InterfaceStubs/driver-test.c
> clang/test/InterfaceStubs/windows.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
> index cdf4a579f431..83b5db3b2530 100644
> --- a/clang/lib/Driver/Driver.cpp
> +++ b/clang/lib/Driver/Driver.cpp
> @@ -292,10 +292,6 @@ phases::ID Driver::getFinalPhase(const DerivedArgList
> &DAL,
>   (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
>  FinalPhase = phases::Compile;
>
> -  // clang interface stubs
> -  } else if ((PhaseArg =
> DAL.getLastArg(options::OPT_emit_interface_stubs))) {
> -FinalPhase = phases::IfsMerge;
> -
>// -S only runs up to the backend.
>} else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
>  FinalPhase = phases::Backend;
> @@ -3502,6 +3498,68 @@ void Driver::BuildActions(Compilation &C,
> DerivedArgList &Args,
>  Actions.push_back(
>  C.MakeAction(MergerInputs, types::TY_Image));
>
> +  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
> +llvm::SmallVector PhaseList;
> +if (Args.hasArg(options::OPT_c)) {
> +  llvm::SmallVector
> CompilePhaseList;
> +  types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
> +  llvm::copy_if(CompilePhaseList, std::back_inserter(PhaseList),
> +[&](phases::ID Phase) { return Phase <=
> phases::Compile; });
> +} else {
> +  types::getCompilationPhases(types::TY_IFS_CPP, PhaseList);
> +}
> +
> +ActionList MergerInputs;
> +
> +for (auto &I : Inputs) {
> +  types::ID InputType = I.first;
> +  const Arg *InputArg = I.second;
> +
> +  // Currently clang and the llvm assembler do not support generating
> symbol
> +  // stubs from assembly, so we skip the input on asm files. For ifs
> files
> +  // we rely on the normal pipeline setup in the pipeline setup code
> above.
> +  if (InputType == types::TY_IFS || InputType == types::TY_PP_Asm ||
> +  InputType == types::TY_Asm)
> +continue;
> +
> +  Action *Current = C.MakeAction(*InputArg, InputType);
> +
> +  for (auto Phase : PhaseList) {
> +switch (Phase) {
> +default:
> +  llvm_unreachable(
> +  "IFS Pipeline can only consist of Compile followed by
> IfsMerge.");
> +case phases::Compile: {
> +  // Only IfsMerge (llvm-ifs) can handle .o files by looking for
> ifs
> +  // files where the .o file is located. The compile action can
> not
> +  // handle this.
> +  if (InputType == types::TY_Object)
> +break;
> +
> +  Current = C.MakeAction(Current,
> types::TY_IFS_CPP);
> +  break;
> +}
> +case phase