<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, May 1, 2017 at 3:06 PM Quentin Colombet via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: qcolombet<br>
Date: Mon May 1 16:53:19 2017<br>
New Revision: 301841<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=301841&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=301841&view=rev</a><br>
Log:<br>
[AArch64] Move GISel accessor initialization from TargetMachine to Subtarget.<br>
<br>
NFC<br>
<br></blockquote><div><br></div><div>Thanks Quentin.</div><div><br></div><div>One inline post-commit review change on the original:</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ // FIXME: At this point, we can't rely on Subtarget having RBI.<br>
+ // It's awkward to mix passing RBI and the Subtarget; should we pass<br>
+ // TII/TRI as well?<br></blockquote><div><br></div><div>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 :)</div><div><br></div><div>Suppose you could also have the accessor return a pointer which could be null if you wanted to conditionalize it?</div><div><br></div><div>*shrug*</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ AArch64GISel->InstSelector.reset(createAArch64InstructionSelector(<br>
+ *static_cast<const AArch64TargetMachine *>(&TM), *this, *RBI));<br>
+<br>
+ AArch64GISel->RegBankInfo.reset(RBI);<br>
+#endif<br>
+ setGISelAccessor(*AArch64GISel);<br>
+}<br>
<br>
const CallLowering *AArch64Subtarget::getCallLowering() const {<br>
assert(GISel && "Access to GlobalISel APIs not set");<br>
<br>
Modified: llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp?rev=301841&r1=301840&r2=301841&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp?rev=301841&r1=301840&r2=301841&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp (original)<br>
+++ llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp Mon May 1 16:53:19 2017<br>
@@ -11,12 +11,7 @@<br>
//===----------------------------------------------------------------------===//<br>
<br>
#include "AArch64.h"<br>
-#include "AArch64CallLowering.h"<br>
-#include "AArch64LegalizerInfo.h"<br>
#include "AArch64MacroFusion.h"<br>
-#ifdef LLVM_BUILD_GLOBAL_ISEL<br>
-#include "AArch64RegisterBankInfo.h"<br>
-#endif<br>
#include "AArch64Subtarget.h"<br>
#include "AArch64TargetMachine.h"<br>
#include "AArch64TargetObjectFile.h"<br>
@@ -25,7 +20,6 @@<br>
#include "llvm/ADT/STLExtras.h"<br>
#include "llvm/ADT/Triple.h"<br>
#include "llvm/Analysis/TargetTransformInfo.h"<br>
-#include "llvm/CodeGen/GlobalISel/GISelAccessor.h"<br>
#include "llvm/CodeGen/GlobalISel/IRTranslator.h"<br>
#include "llvm/CodeGen/GlobalISel/InstructionSelect.h"<br>
#include "llvm/CodeGen/GlobalISel/Legalizer.h"<br>
@@ -222,35 +216,6 @@ AArch64TargetMachine::AArch64TargetMachi<br>
<br>
AArch64TargetMachine::~AArch64TargetMachine() = default;<br>
<br>
-#ifdef LLVM_BUILD_GLOBAL_ISEL<br>
-namespace {<br>
-<br>
-struct AArch64GISelActualAccessor : public GISelAccessor {<br>
- std::unique_ptr<CallLowering> CallLoweringInfo;<br>
- std::unique_ptr<InstructionSelector> InstSelector;<br>
- std::unique_ptr<LegalizerInfo> Legalizer;<br>
- std::unique_ptr<RegisterBankInfo> RegBankInfo;<br>
-<br>
- const CallLowering *getCallLowering() const override {<br>
- return CallLoweringInfo.get();<br>
- }<br>
-<br>
- const InstructionSelector *getInstructionSelector() const override {<br>
- return InstSelector.get();<br>
- }<br>
-<br>
- const LegalizerInfo *getLegalizerInfo() const override {<br>
- return Legalizer.get();<br>
- }<br>
-<br>
- const RegisterBankInfo *getRegBankInfo() const override {<br>
- return RegBankInfo.get();<br>
- }<br>
-};<br>
-<br>
-} // end anonymous namespace<br>
-#endif<br>
-<br>
const AArch64Subtarget *<br>
AArch64TargetMachine::getSubtargetImpl(const Function &F) const {<br>
Attribute CPUAttr = F.getFnAttribute("target-cpu");<br>
@@ -274,26 +239,6 @@ AArch64TargetMachine::getSubtargetImpl(c<br>
resetTargetOptions(F);<br>
I = llvm::make_unique<AArch64Subtarget>(TargetTriple, CPU, FS, *this,<br>
isLittle, ForCodeSize);<br>
-#ifndef LLVM_BUILD_GLOBAL_ISEL<br>
- GISelAccessor *GISel = new GISelAccessor();<br>
-#else<br>
- AArch64GISelActualAccessor *GISel =<br>
- new AArch64GISelActualAccessor();<br>
- GISel->CallLoweringInfo.reset(<br>
- new AArch64CallLowering(*I->getTargetLowering()));<br>
- GISel->Legalizer.reset(new AArch64LegalizerInfo());<br>
-<br>
- auto *RBI = new AArch64RegisterBankInfo(*I->getRegisterInfo());<br>
-<br>
- // FIXME: At this point, we can't rely on Subtarget having RBI.<br>
- // It's awkward to mix passing RBI and the Subtarget; should we pass<br>
- // TII/TRI as well?<br>
- GISel->InstSelector.reset(<br>
- createAArch64InstructionSelector(*this, *I, *RBI));<br>
-<br>
- GISel->RegBankInfo.reset(RBI);<br>
-#endif<br>
- I->setGISelAccessor(*GISel);<br>
}<br>
return I.get();<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>