<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">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><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>