[llvm] r301841 - [AArch64] Move GISel accessor initialization from TargetMachine to Subtarget.

[Eric Christopher via llvm-commits]

On Mon, May 1, 2017 at 3:06 PM Quentin Colombet via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: qcolombet
> Date: Mon May  1 16:53:19 2017
> New Revision: 301841
>
> URL: http://llvm.org/viewvc/llvm-project?rev=301841&view=rev
> Log:
> [AArch64] Move GISel accessor initialization from TargetMachine to
> Subtarget.
>
> NFC
>
>
Thanks Quentin.

One inline post-commit review change on the original:


> +  // FIXME: At this point, we can't rely on Subtarget having RBI.
> +  // It's awkward to mix passing RBI and the Subtarget; should we pass
> +  // TII/TRI as well?
>

This seems like an odd series of comments - especially with creating RBI
above. Is there a GISel target that doesn't have RBI or are you using
something else? Otherwise I'm not sure what the comment is getting us :)

Suppose you could also have the accessor return a pointer which could be
null if you wanted to conditionalize it?

*shrug*

-eric


> +  AArch64GISel->InstSelector.reset(createAArch64InstructionSelector(
> +      *static_cast<const AArch64TargetMachine *>(&TM), *this, *RBI));
> +
> +  AArch64GISel->RegBankInfo.reset(RBI);
> +#endif
> +  setGISelAccessor(*AArch64GISel);
> +}
>
>  const CallLowering *AArch64Subtarget::getCallLowering() const {
>    assert(GISel && "Access to GlobalISel APIs not set");
>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp?rev=301841&r1=301840&r2=301841&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp Mon May  1
> 16:53:19 2017
> @@ -11,12 +11,7 @@
>
>  //===----------------------------------------------------------------------===//
>
>  #include "AArch64.h"
> -#include "AArch64CallLowering.h"
> -#include "AArch64LegalizerInfo.h"
>  #include "AArch64MacroFusion.h"
> -#ifdef LLVM_BUILD_GLOBAL_ISEL
> -#include "AArch64RegisterBankInfo.h"
> -#endif
>  #include "AArch64Subtarget.h"
>  #include "AArch64TargetMachine.h"
>  #include "AArch64TargetObjectFile.h"
> @@ -25,7 +20,6 @@
>  #include "llvm/ADT/STLExtras.h"
>  #include "llvm/ADT/Triple.h"
>  #include "llvm/Analysis/TargetTransformInfo.h"
> -#include "llvm/CodeGen/GlobalISel/GISelAccessor.h"
>  #include "llvm/CodeGen/GlobalISel/IRTranslator.h"
>  #include "llvm/CodeGen/GlobalISel/InstructionSelect.h"
>  #include "llvm/CodeGen/GlobalISel/Legalizer.h"
> @@ -222,35 +216,6 @@ AArch64TargetMachine::AArch64TargetMachi
>
>  AArch64TargetMachine::~AArch64TargetMachine() = default;
>
> -#ifdef LLVM_BUILD_GLOBAL_ISEL
> -namespace {
> -
> -struct AArch64GISelActualAccessor : public GISelAccessor {
> -  std::unique_ptr<CallLowering> CallLoweringInfo;
> -  std::unique_ptr<InstructionSelector> InstSelector;
> -  std::unique_ptr<LegalizerInfo> Legalizer;
> -  std::unique_ptr<RegisterBankInfo> RegBankInfo;
> -
> -  const CallLowering *getCallLowering() const override {
> -    return CallLoweringInfo.get();
> -  }
> -
> -  const InstructionSelector *getInstructionSelector() const override {
> -    return InstSelector.get();
> -  }
> -
> -  const LegalizerInfo *getLegalizerInfo() const override {
> -    return Legalizer.get();
> -  }
> -
> -  const RegisterBankInfo *getRegBankInfo() const override {
> -    return RegBankInfo.get();
> -  }
> -};
> -
> -} // end anonymous namespace
> -#endif
> -
>  const AArch64Subtarget *
>  AArch64TargetMachine::getSubtargetImpl(const Function &F) const {
>    Attribute CPUAttr = F.getFnAttribute("target-cpu");
> @@ -274,26 +239,6 @@ AArch64TargetMachine::getSubtargetImpl(c
>      resetTargetOptions(F);
>      I = llvm::make_unique<AArch64Subtarget>(TargetTriple, CPU, FS, *this,
>                                              isLittle, ForCodeSize);
> -#ifndef LLVM_BUILD_GLOBAL_ISEL
> -    GISelAccessor *GISel = new GISelAccessor();
> -#else
> -    AArch64GISelActualAccessor *GISel =
> -        new AArch64GISelActualAccessor();
> -    GISel->CallLoweringInfo.reset(
> -        new AArch64CallLowering(*I->getTargetLowering()));
> -    GISel->Legalizer.reset(new AArch64LegalizerInfo());
> -
> -    auto *RBI = new AArch64RegisterBankInfo(*I->getRegisterInfo());
> -
> -    // FIXME: At this point, we can't rely on Subtarget having RBI.
> -    // It's awkward to mix passing RBI and the Subtarget; should we pass
> -    // TII/TRI as well?
> -    GISel->InstSelector.reset(
> -        createAArch64InstructionSelector(*this, *I, *RBI));
> -
> -    GISel->RegBankInfo.reset(RBI);
> -#endif
> -    I->setGISelAccessor(*GISel);
>    }
>    return I.get();
>  }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170505/34589f40/attachment.html>