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 ,
> > >               (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 , 
> > > DerivedArgList ,
> > >      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 
> > ,
> >               (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 , 
> > DerivedArgList ,
> >      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  : 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 

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
> ,
>   (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 ,
> DerivedArgList ,
>  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  : 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 phases::IfsMerge: 

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

2019-11-20 Thread Puyan Lotfi via cfe-commits

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 ,
  (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 , DerivedArgList 
,
 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  : 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 phases::IfsMerge: {
+  assert(Phase == PhaseList.back() &&
+ "merging must be final compilation step.");
+  MergerInputs.push_back(Current);
+  Current = nullptr;
+  break;
+}
+}
+  }
+
+  // If we ended with something, add to the output list.
+  if (Current)
+Actions.push_back(Current);
+}
+
+// Add an interface stubs merge action if necessary.
+if (!MergerInputs.empty())
+  Actions.push_back(
+  C.MakeAction(MergerInputs, types::TY_Image));
+  }
+
   // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build a 
custom
   // Compile