r225958 - Use the integrated assembler by default on 32-bit PowerPC and SPARC.

Chandler Carruth chandlerc at google.com
Wed Jan 14 11:45:10 PST 2015


Brad, I think it is completely unacceptable to change the default assembler
behavior of every OS that delegates to Generic_GCC without any warning,
heads up email, or discussion on the lists.

It is especially unacceptable to do so when there are active problems on
build bots and the tests aren't passing. This wasn't the first time this
patch caused a problem either, and you are forcing several other developers
to chase down build bot failures.

Rafael, please revert this and the related patches to return trunk to a
clean state. Also, please make sure that the 3.6 branch is in a clean state
(either prior to the patches or including the reverts).

Brad, please get explicit approval before committing again, get approval
from maintainers of all the impacted OSes, and ensure you track all the
build bots for regressions. When changing the driver's behavior on many
different OSes, this kind of careful and diligent testing is absolutely
necessary.

On Wed, Jan 14, 2015 at 6:10 AM, Rafael EspĂ­ndola <
rafael.espindola at gmail.com> wrote:

> Where was this approved? Have you now managed to run the tests without
> error? How? The openbsd bot is still failing.
>
> You were explicitly reminded that committing without being able to run
> the tests is not OK. Are you doing that again?
>
> On 14 January 2015 at 02:55, Brad Smith <brad at comstyle.com> wrote:
> > Author: brad
> > Date: Wed Jan 14 01:55:36 2015
> > New Revision: 225958
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=225958&view=rev
> > Log:
> > Use the integrated assembler by default on 32-bit PowerPC and SPARC.
> >
> > Modified:
> >     cfe/trunk/lib/Driver/ToolChains.cpp
> >     cfe/trunk/lib/Driver/ToolChains.h
> >     cfe/trunk/test/Driver/freebsd.c
> >     cfe/trunk/test/Driver/gcc_forward.c
> >     cfe/trunk/test/Driver/unknown-gcc-arch.c
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=225958&r1=225957&r2=225958&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Driver/ToolChains.cpp (original)
> > +++ cfe/trunk/lib/Driver/ToolChains.cpp Wed Jan 14 01:55:36 2015
> > @@ -2067,8 +2067,11 @@ bool Generic_GCC::IsIntegratedAssemblerD
> >           getTriple().getArch() == llvm::Triple::armeb ||
> >           getTriple().getArch() == llvm::Triple::thumb ||
> >           getTriple().getArch() == llvm::Triple::thumbeb ||
> > +         getTriple().getArch() == llvm::Triple::ppc ||
> >           getTriple().getArch() == llvm::Triple::ppc64 ||
> >           getTriple().getArch() == llvm::Triple::ppc64le ||
> > +         getTriple().getArch() == llvm::Triple::sparc ||
> > +         getTriple().getArch() == llvm::Triple::sparcv9 ||
> >           getTriple().getArch() == llvm::Triple::systemz;
> >  }
> >
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=225958&r1=225957&r2=225958&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Driver/ToolChains.h (original)
> > +++ cfe/trunk/lib/Driver/ToolChains.h Wed Jan 14 01:55:36 2015
> > @@ -534,17 +534,6 @@ public:
> >      return 2;
> >    }
> >
> > -  virtual bool IsIntegratedAssemblerDefault() const override {
> > -    switch (getTriple().getArch()) {
> > -    case llvm::Triple::ppc:
> > -    case llvm::Triple::sparc:
> > -    case llvm::Triple::sparcv9:
> > -      return true;
> > -    default:
> > -      return Generic_ELF::IsIntegratedAssemblerDefault();
> > -    }
> > -  }
> > -
> >  protected:
> >    Tool *buildAssembler() const override;
> >    Tool *buildLinker() const override;
> > @@ -586,14 +575,6 @@ public:
> >    void
> >    AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
> >                                 llvm::opt::ArgStringList &CC1Args) const
> override;
> > -  bool IsIntegratedAssemblerDefault() const override {
> > -    switch (getTriple().getArch()) {
> > -    case llvm::Triple::ppc:
> > -      return true;
> > -    default:
> > -      return Generic_ELF::IsIntegratedAssemblerDefault();
> > -    }
> > -  }
> >
> >    bool UseSjLjExceptions() const override;
> >    bool isPIEDefault() const override;
> > @@ -618,14 +599,6 @@ public:
> >    bool IsUnwindTablesDefault() const override {
> >      return true;
> >    }
> > -  bool IsIntegratedAssemblerDefault() const override {
> > -    switch (getTriple().getArch()) {
> > -    case llvm::Triple::ppc:
> > -      return true;
> > -    default:
> > -      return Generic_ELF::IsIntegratedAssemblerDefault();
> > -    }
> > -  }
> >
> >  protected:
> >    Tool *buildAssembler() const override;
> >
> > Modified: cfe/trunk/test/Driver/freebsd.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/freebsd.c?rev=225958&r1=225957&r2=225958&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/test/Driver/freebsd.c (original)
> > +++ cfe/trunk/test/Driver/freebsd.c Wed Jan 14 01:55:36 2015
> > @@ -128,11 +128,11 @@
> >  // RUN:   | FileCheck --check-prefix=CHECK-LTO %s
> >  // CHECK-LTO: ld{{.*}}" "-plugin{{.*}}LLVMgold.so
> >
> > -// RUN: %clang -target sparc-unknown-freebsd8 %s -### -fpic 2>&1 \
> > +// RUN: %clang -target sparc-unknown-freebsd8 %s -### -fpic
> -no-integrated-as 2>&1 \
> >  // RUN:   | FileCheck --check-prefix=CHECK-SPARC-PIE %s
> >  // CHECK-SPARC-PIE: as{{.*}}" "-KPIC
> >
> > -// RUN: %clang -mcpu=ultrasparc -target sparc64-unknown-freebsd8 %s
> -### 2>&1 \
> > +// RUN: %clang -mcpu=ultrasparc -target sparc64-unknown-freebsd8 %s
> -### -no-integrated-as 2>&1 \
> >  // RUN:   | FileCheck --check-prefix=CHECK-SPARC-CPU %s
> >  // CHECK-SPARC-CPU: cc1{{.*}}" "-target-cpu" "ultrasparc"
> >  // CHECK-SPARC-CPU: as{{.*}}" "-Av9a
> >
> > Modified: cfe/trunk/test/Driver/gcc_forward.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/gcc_forward.c?rev=225958&r1=225957&r2=225958&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/test/Driver/gcc_forward.c (original)
> > +++ cfe/trunk/test/Driver/gcc_forward.c Wed Jan 14 01:55:36 2015
> > @@ -11,10 +11,6 @@
> >  //
> >  // clang-cc1
> >  // CHECK: "-Wall" "-Wdocumentation"
> > -// CHECK: "-o" "{{[^"]+}}.s"
> > -//
> > -// gnu-as
> > -// CHECK: as{{[^"]*}}"
> >  // CHECK: "-o" "{{[^"]+}}.o"
> >  //
> >  // gcc-ld
> >
> > Modified: cfe/trunk/test/Driver/unknown-gcc-arch.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/unknown-gcc-arch.c?rev=225958&r1=225957&r2=225958&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/test/Driver/unknown-gcc-arch.c (original)
> > +++ cfe/trunk/test/Driver/unknown-gcc-arch.c Wed Jan 14 01:55:36 2015
> > @@ -24,11 +24,11 @@
> >
> >
> >  // RUN: %clang -target powerpc64-unknown-unknown -c -x assembler %s
> -### -m32 \
> > -// RUN: 2>&1 | FileCheck -check-prefix=PPC64-M32 %s
> > +// RUN: -no-integrated-as 2>&1 | FileCheck -check-prefix=PPC64-M32 %s
> >  // PPC64-M32: {{.*as.*-a32}}
> >
> >  // RUN: %clang -target powerpc-unknown-unknown -c -x assembler %s -###
> 2>&1 \
> > -// RUN:   | FileCheck -check-prefix=PPC %s
> > +// RUN: -no-integrated-as | FileCheck -check-prefix=PPC %s
> >  // PPC: {{.*as.*-a32}}
> >
> >  // RUN: %clang -target sparc64-unknown-unknown -no-integrated-as -c -x
> assembler %s -### -m32 2>&1 \
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150114/7f041ad2/attachment.html>


More information about the cfe-commits mailing list