<div dir="ltr">Thank you! </div><br><div class="gmail_quote"><div dir="ltr">On Thu, Apr 27, 2017 at 11:24 AM Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Hi Eric,<div><br></div><div>Catching up with my emails after my paternity leave.</div><div><br></div><div><br><div></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div>On Apr 25, 2017, at 2:57 PM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:</div><br class="m_-6373834264525791603Apple-interchange-newline"><div><div dir="ltr">Adding in Gerolf since he's interested in this stuff too.<div><br></div><div>And a quick ping. :)</div><div><br></div><div>-eric<br><br><div class="gmail_quote"><div dir="ltr">On Fri, Apr 14, 2017 at 10:28 PM Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Hi Quentin,</div><div><br></div><div>I realize this is some definite archaeology, but the code is still there so...</div><div><br></div><div>Taking a look at r265567 I see this:</div></div><div dir="ltr"><div><br></div><div>+#ifndef LLVM_BUILD_GLOBAL_ISEL</div><div>+ AArch64GISelAccessor *GISelAccessor = new AArch64GISelAccessor();</div><div>+#else</div><div>+ AArch64GISelActualAccessor *GISelAccessor =</div><div>+ new AArch64GISelActualAccessor();</div><div>+ GISelAccessor->CallLoweringInfo.reset(</div><div>+ new AArch64CallLowering(*I->getTargetLowering()));</div><div>+ GISelAccessor->RegBankInfo.reset(</div><div>+ new AArch64RegisterBankInfo(*I->getRegisterInfo()));</div><div>+#endif</div><div>+ I->setGISelAccessor(*GISelAccessor);</div><div><br></div></div><div dir="ltr"><div>and I don't think that this is the right place to do this work.</div><div><br></div><div>It seems reasonable that all of this happens inside the subtarget construction rather than after we've constructed a new subtarget. Also means you can simplify a lot of the accesses.</div><div><br></div><div>Thoughts?</div></div></blockquote></div></div></div></div></blockquote><div><br></div></div></div></div><div style="word-wrap:break-word"><div><div><div>Sounds reasonable.</div><div><br></div><div>I could have sworn there was a good reason why I didn’t do that, but it seems I am just stupid!</div><div><br></div><div>I’ll fix that in the coming days.</div><div><br></div><div>Thanks,</div><div>-Quentin</div></div></div></div><div style="word-wrap:break-word"><div><div><br><blockquote type="cite"><div><div dir="ltr"><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Thanks!</div></div><div dir="ltr"><div><br></div><div>-eric</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Apr 6, 2016 at 10:31 AM Quentin Colombet via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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: Wed Apr 6 12:26:03 2016<br>
New Revision: 265567<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=265567&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=265567&view=rev</a><br>
Log:<br>
[AArch64] Teach the subtarget how to get to the RegisterBankInfo.<br>
<br>
Rework the access to GlobalISel APIs to contain how much of<br>
the APIs we need to access for the final executable to build when<br>
GlobalISel is not built.<br>
<br>
This prevents massive usage of ifdefs in various places. Now, all the<br>
GlobalISel ifdefs will be happing only in AArch64TargetMachine.cpp.<br>
<br>
Added:<br>
llvm/trunk/lib/Target/AArch64/AArch64GISelAccessor.h<br>
Modified:<br>
llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp<br>
llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h<br>
llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp<br>
<br>
Added: llvm/trunk/lib/Target/AArch64/AArch64GISelAccessor.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64GISelAccessor.h?rev=265567&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64GISelAccessor.h?rev=265567&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AArch64/AArch64GISelAccessor.h (added)<br>
+++ llvm/trunk/lib/Target/AArch64/AArch64GISelAccessor.h Wed Apr 6 12:26:03 2016<br>
@@ -0,0 +1,33 @@<br>
+//===-- AArch64GISelAccessor.h - AArch64 GISel Accessor ---------*- C++ -*-===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+/// This file declares the API to access the various APIs related<br>
+/// to GlobalISel.<br>
+//<br>
+//===----------------------------------------------------------------------===/<br>
+<br>
+#ifndef LLVM_LIB_TARGET_AARCH64_AARCH64GISELACCESSOR_H<br>
+#define LLVM_LIB_TARGET_AARCH64_AARCH64GISELACCESSOR_H<br>
+<br>
+namespace llvm {<br>
+class CallLowering;<br>
+class RegisterBankInfo;<br>
+<br>
+/// The goal of this helper class is to gather the accessor to all<br>
+/// the APIs related to GlobalISel.<br>
+/// It should be derived to feature an actual accessor to the GISel APIs.<br>
+/// The reason why this is not simply done into the subtarget is to avoid<br>
+/// spreading ifdefs around.<br>
+struct AArch64GISelAccessor {<br>
+ virtual ~AArch64GISelAccessor() {}<br>
+ virtual const CallLowering *getCallLowering() const { return nullptr;}<br>
+ virtual const RegisterBankInfo *getRegBankInfo() const { return nullptr;}<br>
+};<br>
+} // End namespace llvm;<br>
+#endif<br>
<br>
Modified: llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp?rev=265567&r1=265566&r2=265567&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp?rev=265567&r1=265566&r2=265567&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp (original)<br>
+++ llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp Wed Apr 6 12:26:03 2016<br>
@@ -57,12 +57,16 @@ AArch64Subtarget::AArch64Subtarget(const<br>
StrictAlign(false), ReserveX18(TT.isOSDarwin()), IsLittle(LittleEndian),<br>
CPUString(CPU), TargetTriple(TT), FrameLowering(),<br>
InstrInfo(initializeSubtargetDependencies(FS)), TSInfo(),<br>
- TLInfo(TM, *this), CallLoweringInfo(nullptr) {}<br>
+ TLInfo(TM, *this), GISelAccessor() {}<br>
<br>
const CallLowering *AArch64Subtarget::getCallLowering() const {<br>
- if (!CallLoweringInfo)<br>
- CallLoweringInfo.reset(new AArch64CallLowering(TLInfo));<br>
- return CallLoweringInfo.get();<br>
+ assert(GISelAccessor && "Access to GlobalISel APIs not set");<br>
+ return GISelAccessor->getCallLowering();<br>
+}<br>
+<br>
+const RegisterBankInfo *AArch64Subtarget::getRegBankInfo() const {<br>
+ assert(GISelAccessor && "Access to GlobalISel APIs not set");<br>
+ return GISelAccessor->getRegBankInfo();<br>
}<br>
<br>
/// ClassifyGlobalReference - Find the target operand flags that describe<br>
<br>
Modified: llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h?rev=265567&r1=265566&r2=265567&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h?rev=265567&r1=265566&r2=265567&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h (original)<br>
+++ llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h Wed Apr 6 12:26:03 2016<br>
@@ -14,8 +14,8 @@<br>
#ifndef LLVM_LIB_TARGET_AARCH64_AARCH64SUBTARGET_H<br>
#define LLVM_LIB_TARGET_AARCH64_AARCH64SUBTARGET_H<br>
<br>
-#include "AArch64CallLowering.h"<br>
#include "AArch64FrameLowering.h"<br>
+#include "AArch64GISelAccessor.h"<br>
#include "AArch64ISelLowering.h"<br>
#include "AArch64InstrInfo.h"<br>
#include "AArch64RegisterInfo.h"<br>
@@ -82,7 +82,10 @@ protected:<br>
AArch64InstrInfo InstrInfo;<br>
AArch64SelectionDAGInfo TSInfo;<br>
AArch64TargetLowering TLInfo;<br>
- mutable std::unique_ptr<AArch64CallLowering> CallLoweringInfo;<br>
+ /// Gather the accessor points to GlobalISel-related APIs.<br>
+ /// This is used to avoid ifndefs spreading around while GISel is<br>
+ /// an optional library.<br>
+ std::unique_ptr<AArch64GISelAccessor> GISelAccessor;<br>
<br>
private:<br>
/// initializeSubtargetDependencies - Initializes using CPUString and the<br>
@@ -97,6 +100,11 @@ public:<br>
const std::string &FS, const TargetMachine &TM,<br>
bool LittleEndian);<br>
<br>
+ /// This object will take onwership of \p GISelAccessor.<br>
+ void setGISelAccessor(AArch64GISelAccessor &GISelAccessor) {<br>
+ this->GISelAccessor.reset(&GISelAccessor);<br>
+ }<br>
+<br>
const AArch64SelectionDAGInfo *getSelectionDAGInfo() const override {<br>
return &TSInfo;<br>
}<br>
@@ -111,6 +119,7 @@ public:<br>
return &getInstrInfo()->getRegisterInfo();<br>
}<br>
const CallLowering *getCallLowering() const override;<br>
+ const RegisterBankInfo *getRegBankInfo() const override;<br>
const Triple &getTargetTriple() const { return TargetTriple; }<br>
bool enableMachineScheduler() const override { return true; }<br>
bool enablePostRAScheduler() const override {<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=265567&r1=265566&r2=265567&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp?rev=265567&r1=265566&r2=265567&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp (original)<br>
+++ llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp Wed Apr 6 12:26:03 2016<br>
@@ -11,6 +11,8 @@<br>
//===----------------------------------------------------------------------===//<br>
<br>
#include "AArch64.h"<br>
+#include "AArch64CallLowering.h"<br>
+#include "AArch64RegisterBankInfo.h"<br>
#include "AArch64TargetMachine.h"<br>
#include "AArch64TargetObjectFile.h"<br>
#include "AArch64TargetTransformInfo.h"<br>
@@ -154,6 +156,21 @@ AArch64TargetMachine::AArch64TargetMachi<br>
<br>
AArch64TargetMachine::~AArch64TargetMachine() {}<br>
<br>
+#ifdef LLVM_BUILD_GLOBAL_ISEL<br>
+namespace {<br>
+struct AArch64GISelActualAccessor : public AArch64GISelAccessor {<br>
+ std::unique_ptr<CallLowering> CallLoweringInfo;<br>
+ std::unique_ptr<RegisterBankInfo> RegBankInfo;<br>
+ const CallLowering *getCallLowering() const override {<br>
+ return CallLoweringInfo.get();<br>
+ }<br>
+ const RegisterBankInfo *getRegBankInfo() const override {<br>
+ return RegBankInfo.get();<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>
@@ -174,6 +191,17 @@ AArch64TargetMachine::getSubtargetImpl(c<br>
resetTargetOptions(F);<br>
I = llvm::make_unique<AArch64Subtarget>(TargetTriple, CPU, FS, *this,<br>
isLittle);<br>
+#ifndef LLVM_BUILD_GLOBAL_ISEL<br>
+ AArch64GISelAccessor *GISelAccessor = new AArch64GISelAccessor();<br>
+#else<br>
+ AArch64GISelActualAccessor *GISelAccessor =<br>
+ new AArch64GISelActualAccessor();<br>
+ GISelAccessor->CallLoweringInfo.reset(<br>
+ new AArch64CallLowering(*I->getTargetLowering()));<br>
+ GISelAccessor->RegBankInfo.reset(<br>
+ new AArch64RegisterBankInfo(*I->getRegisterInfo()));<br>
+#endif<br>
+ I->setGISelAccessor(*GISelAccessor);<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></blockquote></div></div></div>
</div></blockquote></div></div></div></blockquote></div>