<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">+Ahmed<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On May 4, 2017, at 9:36 PM, Eric Christopher <<a href="mailto:echristo@gmail.com" class="">echristo@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Mon, May 1, 2017 at 3:06 PM Quentin Colombet via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">Author: qcolombet<br class="">Date: Mon May 1 16:53:19 2017<br class="">New Revision: 301841<br class=""><br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=301841&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=301841&view=rev</a><br class="">Log:<br class="">[AArch64] Move GISel accessor initialization from TargetMachine to Subtarget.<br class=""><br class="">NFC<br class=""><br class=""></blockquote><div class=""><br class=""></div><div class="">Thanks Quentin.</div><div class=""><br class=""></div><div class="">One inline post-commit review change on the original:</div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">+ // FIXME: At this point, we can't rely on Subtarget having RBI.<br class="">+ // It's awkward to mix passing RBI and the Subtarget; should we pass<br class="">+ // TII/TRI as well?<br class=""></blockquote><div class=""><br class=""></div><div class="">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?</div></div></div></div></blockquote><div><br class=""></div><div>Not that I am aware off.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote"><div class=""> Otherwise I'm not sure what the comment is getting us :)</div></div></div></div></blockquote><div><br class=""></div><div>Ahmed, could you comment on this? I admit I am as puzzled as Eric here.</div><div><br class=""></div><div>Cheers,</div><div>-Quentin</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote"><div class=""><br class=""></div><div class="">Suppose you could also have the accessor return a pointer which could be null if you wanted to conditionalize it?</div><div class=""><br class=""></div><div class="">*shrug*</div><div class=""><br class=""></div><div class="">-eric</div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">+ AArch64GISel->InstSelector.reset(createAArch64InstructionSelector(<br class="">+ *static_cast<const AArch64TargetMachine *>(&TM), *this, *RBI));<br class="">+<br class="">+ AArch64GISel->RegBankInfo.reset(RBI);<br class="">+#endif<br class="">+ setGISelAccessor(*AArch64GISel);<br class="">+}<br class=""><br class=""> const CallLowering *AArch64Subtarget::getCallLowering() const {<br class=""> assert(GISel && "Access to GlobalISel APIs not set");<br class=""><br class="">Modified: llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp<br class="">URL:<span class="Apple-converted-space"> </span><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" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp?rev=301841&r1=301840&r2=301841&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp (original)<br class="">+++ llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp Mon May 1 16:53:19 2017<br class="">@@ -11,12 +11,7 @@<br class=""> //===----------------------------------------------------------------------===//<br class=""><br class=""> #include "AArch64.h"<br class="">-#include "AArch64CallLowering.h"<br class="">-#include "AArch64LegalizerInfo.h"<br class=""> #include "AArch64MacroFusion.h"<br class="">-#ifdef LLVM_BUILD_GLOBAL_ISEL<br class="">-#include "AArch64RegisterBankInfo.h"<br class="">-#endif<br class=""> #include "AArch64Subtarget.h"<br class=""> #include "AArch64TargetMachine.h"<br class=""> #include "AArch64TargetObjectFile.h"<br class="">@@ -25,7 +20,6 @@<br class=""> #include "llvm/ADT/STLExtras.h"<br class=""> #include "llvm/ADT/Triple.h"<br class=""> #include "llvm/Analysis/TargetTransformInfo.h"<br class="">-#include "llvm/CodeGen/GlobalISel/GISelAccessor.h"<br class=""> #include "llvm/CodeGen/GlobalISel/IRTranslator.h"<br class=""> #include "llvm/CodeGen/GlobalISel/InstructionSelect.h"<br class=""> #include "llvm/CodeGen/GlobalISel/Legalizer.h"<br class="">@@ -222,35 +216,6 @@ AArch64TargetMachine::AArch64TargetMachi<br class=""><br class=""> AArch64TargetMachine::~AArch64TargetMachine() = default;<br class=""><br class="">-#ifdef LLVM_BUILD_GLOBAL_ISEL<br class="">-namespace {<br class="">-<br class="">-struct AArch64GISelActualAccessor : public GISelAccessor {<br class="">- std::unique_ptr<CallLowering> CallLoweringInfo;<br class="">- std::unique_ptr<InstructionSelector> InstSelector;<br class="">- std::unique_ptr<LegalizerInfo> Legalizer;<br class="">- std::unique_ptr<RegisterBankInfo> RegBankInfo;<br class="">-<br class="">- const CallLowering *getCallLowering() const override {<br class="">- return CallLoweringInfo.get();<br class="">- }<br class="">-<br class="">- const InstructionSelector *getInstructionSelector() const override {<br class="">- return InstSelector.get();<br class="">- }<br class="">-<br class="">- const LegalizerInfo *getLegalizerInfo() const override {<br class="">- return Legalizer.get();<br class="">- }<br class="">-<br class="">- const RegisterBankInfo *getRegBankInfo() const override {<br class="">- return RegBankInfo.get();<br class="">- }<br class="">-};<br class="">-<br class="">-} // end anonymous namespace<br class="">-#endif<br class="">-<br class=""> const AArch64Subtarget *<br class=""> AArch64TargetMachine::getSubtargetImpl(const Function &F) const {<br class=""> Attribute CPUAttr = F.getFnAttribute("target-cpu");<br class="">@@ -274,26 +239,6 @@ AArch64TargetMachine::getSubtargetImpl(c<br class=""> resetTargetOptions(F);<br class=""> I = llvm::make_unique<AArch64Subtarget>(TargetTriple, CPU, FS, *this,<br class=""> isLittle, ForCodeSize);<br class="">-#ifndef LLVM_BUILD_GLOBAL_ISEL<br class="">- GISelAccessor *GISel = new GISelAccessor();<br class="">-#else<br class="">- AArch64GISelActualAccessor *GISel =<br class="">- new AArch64GISelActualAccessor();<br class="">- GISel->CallLoweringInfo.reset(<br class="">- new AArch64CallLowering(*I->getTargetLowering()));<br class="">- GISel->Legalizer.reset(new AArch64LegalizerInfo());<br class="">-<br class="">- auto *RBI = new AArch64RegisterBankInfo(*I->getRegisterInfo());<br class="">-<br class="">- // FIXME: At this point, we can't rely on Subtarget having RBI.<br class="">- // It's awkward to mix passing RBI and the Subtarget; should we pass<br class="">- // TII/TRI as well?<br class="">- GISel->InstSelector.reset(<br class="">- createAArch64InstructionSelector(*this, *I, *RBI));<br class="">-<br class="">- GISel->RegBankInfo.reset(RBI);<br class="">-#endif<br class="">- I->setGISelAccessor(*GISel);<br class=""> }<br class=""> return I.get();<br class=""> }<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></blockquote></div></div></div></blockquote></div><br class=""></div></body></html>