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 <pu...@puyan.org> 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 <arpha...@gmail.com> 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 
> > <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<IfsMergeJobAction>(MergerInputs, types::TY_Image));
> > > 

> > > +  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
> > > +    llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> PhaseList;
> > > +    if (Args.hasArg(options::OPT_c)) {
> > > +      llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> 
> > > 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<InputAction>(*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<CompileJobAction>(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<IfsMergeJobAction>(MergerInputs, 
> > > types::TY_Image));
> > > +  }
> > > +
> > >    // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build 
> > > a custom
> > >    // Compile phase that prints out supported cpu models and quits.
> > >    if (Arg *A = Args.getLastArg(options::OPT_print_supported_cpus)) {
> > > @@ -3603,8 +3661,6 @@ Action *Driver::ConstructPhaseAction(
> > >        return C.MakeAction<CompileJobAction>(Input, types::TY_ModuleFile);
> > >      if (Args.hasArg(options::OPT_verify_pch))
> > >        return C.MakeAction<VerifyPCHJobAction>(Input, types::TY_Nothing);
> > > -    if (Args.hasArg(options::OPT_emit_interface_stubs))
> > > -      return C.MakeAction<CompileJobAction>(Input, types::TY_IFS_CPP);
> > >      return C.MakeAction<CompileJobAction>(Input, types::TY_LLVM_BC);
> > >    }
> > >    case phases::Backend: {
> > > @@ -3633,11 +3689,16 @@ void Driver::BuildJobs(Compilation &C) const {
> > >    Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o);
> > > 

> > >    // It is an error to provide a -o option if we are making multiple 
> > > output
> > > -  // files.
> > > +  // files. There is one exception, IfsMergeJob: when generating 
> > > interface stubs
> > > +  // enabled we want to be able to generate the stub file at the same 
> > > time that
> > > +  // we generate the real library/a.out. So when a .o, .so, etc are the 
> > > output,
> > > +  // with clang interface stubs there will also be a .ifs and .ifso at 
> > > the same
> > > +  // location.
> > >    if (FinalOutput) {
> > >      unsigned NumOutputs = 0;
> > >      for (const Action *A : C.getActions())
> > > -      if (A->getType() != types::TY_Nothing)
> > > +      if (A->getType() != types::TY_Nothing &&
> > > +          A->getKind() != Action::IfsMergeJobClass)
> > >          ++NumOutputs;
> > > 

> > >      if (NumOutputs > 1) {
> > > 

> > > diff  --git a/clang/lib/Driver/ToolChains/InterfaceStubs.cpp 
> > > b/clang/lib/Driver/ToolChains/InterfaceStubs.cpp
> > > index 6677843b2c53..f441f4787097 100644
> > > --- a/clang/lib/Driver/ToolChains/InterfaceStubs.cpp
> > > +++ b/clang/lib/Driver/ToolChains/InterfaceStubs.cpp
> > > @@ -9,6 +9,7 @@
> > >  #include "InterfaceStubs.h"
> > >  #include "CommonArgs.h"
> > >  #include "clang/Driver/Compilation.h"
> > > +#include "llvm/Support/Path.h"
> > > 

> > >  namespace clang {
> > >  namespace driver {
> > > @@ -21,13 +22,36 @@ void Merger::ConstructJob(Compilation &C, const 
> > > JobAction &JA,
> > >    std::string Merger = getToolChain().GetProgramPath(getShortName());
> > >    llvm::opt::ArgStringList CmdArgs;
> > >    CmdArgs.push_back("-action");
> > > -  CmdArgs.push_back(Args.getLastArg(options::OPT_emit_merged_ifs)
> > > -                        ? "write-ifs"
> > > -                        : "write-bin");
> > > +  const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
> > > +  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
> > >    CmdArgs.push_back("-o");
> > > -  CmdArgs.push_back(Output.getFilename());
> > > -  for (const auto &Input : Inputs)
> > > -    CmdArgs.push_back(Input.getFilename());
> > > +
> > > +  // Normally we want to write to a side-car file ending in ".ifso" so 
> > > for
> > > +  // example if `clang -emit-interface-stubs -shared -o libhello.so` were
> > > +  // invoked then we would like to get libhello.so and libhello.ifso. If 
> > > the
> > > +  // stdout stream is given as the output file (ie `-o -`), that is the 
> > > one
> > > +  // exception where we will just append to the same filestream as the 
> > > normal
> > > +  // output.
> > > +  SmallString<128> OutputFilename(Output.getFilename());
> > > +  if (OutputFilename != "-") {
> > > +    if (Args.hasArg(options::OPT_shared))
> > > +      llvm::sys::path::replace_extension(OutputFilename,
> > > +                                         (WriteBin ? "ifso" : "ifs"));
> > > +    else
> > > +      OutputFilename += (WriteBin ? ".ifso" : ".ifs");
> > > +  }
> > > +
> > > +  CmdArgs.push_back(Args.MakeArgString(OutputFilename.c_str()));
> > > +
> > > +  // Here we append the input files. If the input files are object 
> > > files, then
> > > +  // we look for .ifs files present in the same location as the object 
> > > files.
> > > +  for (const auto &Input : Inputs) {
> > > +    SmallString<128> InputFilename(Input.getFilename());
> > > +    if (Input.getType() == types::TY_Object)
> > > +      llvm::sys::path::replace_extension(InputFilename, ".ifs");
> > > +    CmdArgs.push_back(Args.MakeArgString(InputFilename.c_str()));
> > > +  }
> > > +
> > >    C.addCommand(std::make_unique<Command>(JA, *this, 
> > > Args.MakeArgString(Merger),
> > >                                           CmdArgs, Inputs));
> > >  }
> > > 

> > > diff  --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
> > > index 0e14e3d63fab..7d83be2521e7 100644
> > > --- a/clang/lib/Driver/Types.cpp
> > > +++ b/clang/lib/Driver/Types.cpp
> > > @@ -330,22 +330,6 @@ void types::getCompilationPhases(const 
> > > clang::driver::Driver &Driver,
> > >      llvm::copy_if(PhaseList, std::back_inserter(P),
> > >                    [](phases::ID Phase) { return Phase <= 
> > > phases::Precompile; });
> > > 

> > > -  // Treat Interface Stubs like its own compilation mode.
> > > -  else if (DAL.getLastArg(options::OPT_emit_interface_stubs)) {
> > > -    llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> 
> > > IfsModePhaseList;
> > > -    llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> &PL = 
> > > PhaseList;
> > > -    phases::ID LastPhase = phases::IfsMerge;
> > > -    if (Id != types::TY_IFS) {
> > > -      if (DAL.hasArg(options::OPT_c))
> > > -        LastPhase = phases::Compile;
> > > -      PL = IfsModePhaseList;
> > > -      types::getCompilationPhases(types::TY_IFS_CPP, PL);
> > > -    }
> > > -    llvm::copy_if(PL, std::back_inserter(P), [&](phases::ID Phase) {
> > > -      return Phase <= LastPhase;
> > > -    });
> > > -  }
> > > -
> > >    // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
> > >    else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
> > >             DAL.getLastArg(options::OPT_print_supported_cpus) ||
> > > 

> > > diff  --git a/clang/test/InterfaceStubs/driver-test.c 
> > > b/clang/test/InterfaceStubs/driver-test.c
> > > index d4e50295411a..9ca5577a5c20 100644
> > > --- a/clang/test/InterfaceStubs/driver-test.c
> > > +++ b/clang/test/InterfaceStubs/driver-test.c
> > > @@ -1,7 +1,9 @@
> > >  // REQUIRES: x86-registered-target
> > > +// REQUIRES: shell
> > > 

> > > -// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1.so 
> > > -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
> > > -// RUN: llvm-nm %t1.so 2>&1 | FileCheck --check-prefix=CHECK-IFS %s
> > > +// RUN: mkdir -p %t; cd %t
> > > +// RUN: %clang -target x86_64-unknown-linux-gnu -x c -S 
> > > -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
> > > +// RUN: llvm-nm %t/a.out.ifso 2>&1 | FileCheck --check-prefix=CHECK-IFS 
> > > %s
> > > 

> > >  // CHECK-IFS-DAG: data
> > >  // CHECK-IFS-DAG: foo
> > > 

> > > diff  --git a/clang/test/InterfaceStubs/driver-test2.c 
> > > b/clang/test/InterfaceStubs/driver-test2.c
> > > new file mode 100644
> > > index 000000000000..c3a3b31b212d
> > > --- /dev/null
> > > +++ b/clang/test/InterfaceStubs/driver-test2.c
> > > @@ -0,0 +1,16 @@
> > > +// REQUIRES: x86-registered-target
> > > +// REQUIRES: shell
> > > +
> > > +// RUN: mkdir -p %t; cd %t
> > > +// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs 
> > > \
> > > +// RUN:   %s %S/object.c %S/weak.cpp
> > > +// RUN: %clang -emit-interface-stubs -emit-merged-ifs \
> > > +// RUN:   %t/driver-test2.o %t/object.o %t/weak.o -S -o - 2>&1 | 
> > > FileCheck %s
> > > +
> > > +// CHECK-DAG: data
> > > +// CHECK-DAG: bar
> > > +// CHECK-DAG: strongFunc
> > > +// CHECK-DAG: weakFunc
> > > +
> > > +int bar(int a) { return a; }
> > > +int main() { return 0; }
> > > 

> > > diff  --git a/clang/test/InterfaceStubs/ppc.cpp 
> > > b/clang/test/InterfaceStubs/ppc.cpp
> > > new file mode 100644
> > > index 000000000000..9a91697d9506
> > > --- /dev/null
> > > +++ b/clang/test/InterfaceStubs/ppc.cpp
> > > @@ -0,0 +1,14 @@
> > > +// REQUIRES: powerpc-registered-target
> > > +
> > > +// RUN: %clang -x c++ -target powerpc64le-unknown-linux-gnu -o - %s \
> > > +// RUN:   -emit-interface-stubs -emit-merged-ifs -S | \
> > > +// RUN: FileCheck -check-prefix=CHECK-IFS %s
> > > +
> > > + // CHECK-IFS: --- !experimental-ifs-v1
> > > + // CHECK-IFS: IfsVersion:      1.0
> > > + // CHECK-IFS: Triple: powerpc64le
> > > + // CHECK-IFS: Symbols:
> > > + // CHECK-IFS:   _Z8helloPPCv: { Type: Func }
> > > + // CHECK-IFS: ...
> > > +
> > > +int helloPPC();
> > > 

> > > diff  --git a/clang/test/InterfaceStubs/windows.cpp 
> > > b/clang/test/InterfaceStubs/windows.cpp
> > > index 9ccb771a2f39..c81c702861e4 100644
> > > --- a/clang/test/InterfaceStubs/windows.cpp
> > > +++ b/clang/test/InterfaceStubs/windows.cpp
> > > @@ -1,6 +1,7 @@
> > >  // REQUIRES: x86-registered-target
> > >  // RUN: %clang_cc1 -triple x86_64-windows-msvc -o - %s 
> > > -emit-interface-stubs | FileCheck -check-prefix=CHECK-CC1 %s
> > > -// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs 
> > > -emit-merged-ifs | FileCheck -check-prefix=CHECK-IFS %s
> > > +// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs 
> > > -emit-merged-ifs -S | FileCheck -check-prefix=CHECK-IFS %s
> > > +// note: -S is added here to prevent clang from invoking link.exe
> > > 

> > >  // CHECK-CC1: Symbols:
> > >  // CHECK-CC1-NEXT: ?helloWindowsMsvc@@YAHXZ
> > > 

> > > _______________________________________________
> > > cfe-commits mailing list
> > > cfe-commits@lists.llvm.org
> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Attachment: publickey - puyan@puyan.org - 0xD3F77200.asc
Description: application/pgp-keys

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to