<div dir="ltr">Sorry about that, I should have included a link to the review thread in the messages for all of these commits, as it's difficult to search for it using just the individual commit messages.<div><br></div><div>Alex</div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-08-13 4:01 GMT-07:00 Daniel Sanders <span dir="ltr"><<a href="mailto:Daniel.Sanders@imgtec.com" target="_blank">Daniel.Sanders@imgtec.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I've just found the thread (<a href="http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150810/292871.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150810/292871.html</a>). I couldn't find it before because the subject line is very different and there was no link.<br>
<span class="">________________________________________<br>
From: llvm-commits [<a href="mailto:llvm-commits-bounces@lists.llvm.org">llvm-commits-bounces@lists.llvm.org</a>] on behalf of Daniel Sanders via llvm-commits [<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>]<br>
</span>Sent: 13 August 2015 11:44<br>
<div class="HOEnZb"><div class="h5">To: Alex Lorenz; <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
Subject: RE: [llvm] r244698 - PseudoSourceValue: Transform the mips subclass to target independent subclasses<br>
<br>
I've now read through this patch and it LGTM. We just need to address the issue of the commit saying it has been reviewed and the apparent lack of a review thread.<br>
________________________________________<br>
From: llvm-commits [<a href="mailto:llvm-commits-bounces@lists.llvm.org">llvm-commits-bounces@lists.llvm.org</a>] on behalf of Daniel Sanders via llvm-commits [<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>]<br>
Sent: 13 August 2015 11:05<br>
To: Alex Lorenz; <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
Subject: RE: [llvm] r244698 - PseudoSourceValue: Transform the mips subclass to target independent subclasses<br>
<br>
Hi,<br>
<br>
Where was this reviewed? I can't find the thread on the list and there doesn't seem to be a Phabricator review either.<br>
________________________________________<br>
From: llvm-commits [<a href="mailto:llvm-commits-bounces@lists.llvm.org">llvm-commits-bounces@lists.llvm.org</a>] on behalf of Alex Lorenz via llvm-commits [<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>]<br>
Sent: 12 August 2015 00:23<br>
To: <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
Subject: [llvm] r244698 - PseudoSourceValue: Transform the mips subclass to target independent subclasses<br>
<br>
Author: arphaman<br>
Date: Tue Aug 11 18:23:17 2015<br>
New Revision: 244698<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=244698&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=244698&view=rev</a><br>
Log:<br>
PseudoSourceValue: Transform the mips subclass to target independent subclasses<br>
<br>
This commit transforms the mips-specific 'MipsCallEntry' subclass of the<br>
'PseudoSourceValue' class into two, target-independent subclasses named<br>
'GlobalValuePseudoSourceValue' and 'ExternalSymbolPseudoSourceValue'.<br>
<br>
This change makes it easier to serialize the pseudo source values by removing<br>
target-specific pseudo source values.<br>
<br>
Reviewers: Akira Hatanaka<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/CodeGen/PseudoSourceValue.h<br>
llvm/trunk/lib/CodeGen/PseudoSourceValue.cpp<br>
llvm/trunk/lib/Target/Mips/MipsMachineFunction.cpp<br>
llvm/trunk/lib/Target/Mips/MipsMachineFunction.h<br>
<br>
Modified: llvm/trunk/include/llvm/CodeGen/PseudoSourceValue.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/PseudoSourceValue.h?rev=244698&r1=244697&r2=244698&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/PseudoSourceValue.h?rev=244698&r1=244697&r2=244698&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/CodeGen/PseudoSourceValue.h (original)<br>
+++ llvm/trunk/include/llvm/CodeGen/PseudoSourceValue.h Tue Aug 11 18:23:17 2015<br>
@@ -14,7 +14,10 @@<br>
#ifndef LLVM_CODEGEN_PSEUDOSOURCEVALUE_H<br>
#define LLVM_CODEGEN_PSEUDOSOURCEVALUE_H<br>
<br>
+#include "llvm/ADT/StringMap.h"<br>
+#include "llvm/IR/GlobalValue.h"<br>
#include "llvm/IR/Value.h"<br>
+#include "llvm/IR/ValueMap.h"<br>
#include <map><br>
<br>
namespace llvm {<br>
@@ -30,7 +33,15 @@ raw_ostream &operator<<(raw_ostream &OS,<br>
/// below the stack frame (e.g., argument space), or constant pool.<br>
class PseudoSourceValue {<br>
public:<br>
- enum PSVKind { Stack, GOT, JumpTable, ConstantPool, FixedStack, MipsPSV };<br>
+ enum PSVKind {<br>
+ Stack,<br>
+ GOT,<br>
+ JumpTable,<br>
+ ConstantPool,<br>
+ FixedStack,<br>
+ GlobalValueCallEntry,<br>
+ ExternalSymbolCallEntry<br>
+ };<br>
<br>
private:<br>
PSVKind Kind;<br>
@@ -90,10 +101,45 @@ public:<br>
int getFrameIndex() const { return FI; }<br>
};<br>
<br>
+class CallEntryPseudoSourceValue : public PseudoSourceValue {<br>
+protected:<br>
+ CallEntryPseudoSourceValue(PSVKind Kind);<br>
+<br>
+public:<br>
+ bool isConstant(const MachineFrameInfo *) const override;<br>
+ bool isAliased(const MachineFrameInfo *) const override;<br>
+ bool mayAlias(const MachineFrameInfo *) const override;<br>
+};<br>
+<br>
+/// A specialized pseudo soruce value for holding GlobalValue values.<br>
+class GlobalValuePseudoSourceValue : public CallEntryPseudoSourceValue {<br>
+ const GlobalValue *GV;<br>
+<br>
+public:<br>
+ GlobalValuePseudoSourceValue(const GlobalValue *GV);<br>
+<br>
+ const GlobalValue *getValue() const { return GV; }<br>
+};<br>
+<br>
+/// A specialized pseudo source value for holding external symbol values.<br>
+class ExternalSymbolPseudoSourceValue : public CallEntryPseudoSourceValue {<br>
+ const char *ES;<br>
+<br>
+public:<br>
+ ExternalSymbolPseudoSourceValue(const char *ES);<br>
+<br>
+ const char *getSymbol() const { return ES; }<br>
+};<br>
+<br>
/// Manages creation of pseudo source values.<br>
class PseudoSourceValueManager {<br>
const PseudoSourceValue StackPSV, GOTPSV, JumpTablePSV, ConstantPoolPSV;<br>
std::map<int, std::unique_ptr<FixedStackPseudoSourceValue>> FSValues;<br>
+ StringMap<std::unique_ptr<const ExternalSymbolPseudoSourceValue>><br>
+ ExternalCallEntries;<br>
+ ValueMap<const GlobalValue *,<br>
+ std::unique_ptr<const GlobalValuePseudoSourceValue>><br>
+ GlobalCallEntries;<br>
<br>
public:<br>
PseudoSourceValueManager();<br>
@@ -118,6 +164,10 @@ public:<br>
/// Return a pseudo source value referencing a fixed stack frame entry,<br>
/// e.g., a spill slot.<br>
const PseudoSourceValue *getFixedStack(int FI);<br>
+<br>
+ const PseudoSourceValue *getGlobalValueCallEntry(const GlobalValue *GV);<br>
+<br>
+ const PseudoSourceValue *getExternalSymbolCallEntry(const char *ES);<br>
};<br>
<br>
} // end namespace llvm<br>
<br>
Modified: llvm/trunk/lib/CodeGen/PseudoSourceValue.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PseudoSourceValue.cpp?rev=244698&r1=244697&r2=244698&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PseudoSourceValue.cpp?rev=244698&r1=244697&r2=244698&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/PseudoSourceValue.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/PseudoSourceValue.cpp Tue Aug 11 18:23:17 2015<br>
@@ -24,7 +24,8 @@<br>
using namespace llvm;<br>
<br>
static const char *const PSVNames[] = {<br>
- "Stack", "GOT", "JumpTable", "ConstantPool", "FixedStack", "MipsCallEntry"};<br>
+ "Stack", "GOT", "JumpTable", "ConstantPool", "FixedStack",<br>
+ "GlobalValueCallEntry", "ExternalSymbolCallEntry"};<br>
<br>
PseudoSourceValue::PseudoSourceValue(PSVKind Kind) : Kind(Kind) {}<br>
<br>
@@ -76,6 +77,28 @@ void FixedStackPseudoSourceValue::printC<br>
OS << "FixedStack" << FI;<br>
}<br>
<br>
+CallEntryPseudoSourceValue::CallEntryPseudoSourceValue(PSVKind Kind)<br>
+ : PseudoSourceValue(Kind) {}<br>
+<br>
+bool CallEntryPseudoSourceValue::isConstant(const MachineFrameInfo *) const {<br>
+ return false;<br>
+}<br>
+<br>
+bool CallEntryPseudoSourceValue::isAliased(const MachineFrameInfo *) const {<br>
+ return false;<br>
+}<br>
+<br>
+bool CallEntryPseudoSourceValue::mayAlias(const MachineFrameInfo *) const {<br>
+ return false;<br>
+}<br>
+<br>
+GlobalValuePseudoSourceValue::GlobalValuePseudoSourceValue(<br>
+ const GlobalValue *GV)<br>
+ : CallEntryPseudoSourceValue(GlobalValueCallEntry), GV(GV) {}<br>
+<br>
+ExternalSymbolPseudoSourceValue::ExternalSymbolPseudoSourceValue(const char *ES)<br>
+ : CallEntryPseudoSourceValue(ExternalSymbolCallEntry), ES(ES) {}<br>
+<br>
PseudoSourceValueManager::PseudoSourceValueManager()<br>
: StackPSV(PseudoSourceValue::Stack), GOTPSV(PseudoSourceValue::GOT),<br>
JumpTablePSV(PseudoSourceValue::JumpTable),<br>
@@ -101,3 +124,21 @@ const PseudoSourceValue *PseudoSourceVal<br>
V = llvm::make_unique<FixedStackPseudoSourceValue>(FI);<br>
return V.get();<br>
}<br>
+<br>
+const PseudoSourceValue *<br>
+PseudoSourceValueManager::getGlobalValueCallEntry(const GlobalValue *GV) {<br>
+ std::unique_ptr<const GlobalValuePseudoSourceValue> &E =<br>
+ GlobalCallEntries[GV];<br>
+ if (!E)<br>
+ E = llvm::make_unique<GlobalValuePseudoSourceValue>(GV);<br>
+ return E.get();<br>
+}<br>
+<br>
+const PseudoSourceValue *<br>
+PseudoSourceValueManager::getExternalSymbolCallEntry(const char *ES) {<br>
+ std::unique_ptr<const ExternalSymbolPseudoSourceValue> &E =<br>
+ ExternalCallEntries[ES];<br>
+ if (!E)<br>
+ E = llvm::make_unique<ExternalSymbolPseudoSourceValue>(ES);<br>
+ return E.get();<br>
+}<br>
<br>
Modified: llvm/trunk/lib/Target/Mips/MipsMachineFunction.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsMachineFunction.cpp?rev=244698&r1=244697&r2=244698&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsMachineFunction.cpp?rev=244698&r1=244697&r2=244698&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/Mips/MipsMachineFunction.cpp (original)<br>
+++ llvm/trunk/lib/Target/Mips/MipsMachineFunction.cpp Tue Aug 11 18:23:17 2015<br>
@@ -24,43 +24,6 @@ static cl::opt<bool><br>
FixGlobalBaseReg("mips-fix-global-base-reg", cl::Hidden, cl::init(true),<br>
cl::desc("Always use $gp as the global base register."));<br>
<br>
-// class MipsCallEntry.<br>
-MipsCallEntry::MipsCallEntry(StringRef N) : PseudoSourceValue(MipsPSV) {<br>
-#ifndef NDEBUG<br>
- Name = N;<br>
- Val = nullptr;<br>
-#endif<br>
-}<br>
-<br>
-MipsCallEntry::MipsCallEntry(const GlobalValue *V)<br>
- : PseudoSourceValue(MipsPSV) {<br>
-#ifndef NDEBUG<br>
- Val = V;<br>
-#endif<br>
-}<br>
-<br>
-bool MipsCallEntry::isConstant(const MachineFrameInfo *) const {<br>
- return false;<br>
-}<br>
-<br>
-bool MipsCallEntry::isAliased(const MachineFrameInfo *) const {<br>
- return false;<br>
-}<br>
-<br>
-bool MipsCallEntry::mayAlias(const MachineFrameInfo *) const {<br>
- return false;<br>
-}<br>
-<br>
-void MipsCallEntry::printCustom(raw_ostream &O) const {<br>
- O << "MipsCallEntry: ";<br>
-#ifndef NDEBUG<br>
- if (Val)<br>
- O << Val->getName();<br>
- else<br>
- O << Name;<br>
-#endif<br>
-}<br>
-<br>
MipsFunctionInfo::~MipsFunctionInfo() {}<br>
<br>
bool MipsFunctionInfo::globalBaseRegSet() const {<br>
@@ -117,22 +80,12 @@ bool MipsFunctionInfo::isEhDataRegFI(int<br>
|| FI == EhDataRegFI[2] || FI == EhDataRegFI[3]);<br>
}<br>
<br>
-MachinePointerInfo MipsFunctionInfo::callPtrInfo(StringRef Name) {<br>
- std::unique_ptr<const MipsCallEntry> &E = ExternalCallEntries[Name];<br>
-<br>
- if (!E)<br>
- E = llvm::make_unique<MipsCallEntry>(Name);<br>
-<br>
- return MachinePointerInfo(E.get());<br>
+MachinePointerInfo MipsFunctionInfo::callPtrInfo(const char *ES) {<br>
+ return MachinePointerInfo(MF.getPSVManager().getExternalSymbolCallEntry(ES));<br>
}<br>
<br>
-MachinePointerInfo MipsFunctionInfo::callPtrInfo(const GlobalValue *Val) {<br>
- std::unique_ptr<const MipsCallEntry> &E = GlobalCallEntries[Val];<br>
-<br>
- if (!E)<br>
- E = llvm::make_unique<MipsCallEntry>(Val);<br>
-<br>
- return MachinePointerInfo(E.get());<br>
+MachinePointerInfo MipsFunctionInfo::callPtrInfo(const GlobalValue *GV) {<br>
+ return MachinePointerInfo(MF.getPSVManager().getGlobalValueCallEntry(GV));<br>
}<br>
<br>
int MipsFunctionInfo::getMoveF64ViaSpillFI(const TargetRegisterClass *RC) {<br>
<br>
Modified: llvm/trunk/lib/Target/Mips/MipsMachineFunction.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsMachineFunction.h?rev=244698&r1=244697&r2=244698&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsMachineFunction.h?rev=244698&r1=244697&r2=244698&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/Mips/MipsMachineFunction.h (original)<br>
+++ llvm/trunk/lib/Target/Mips/MipsMachineFunction.h Tue Aug 11 18:23:17 2015<br>
@@ -15,12 +15,10 @@<br>
#define LLVM_LIB_TARGET_MIPS_MIPSMACHINEFUNCTION_H<br>
<br>
#include "Mips16HardFloatInfo.h"<br>
-#include "llvm/ADT/StringMap.h"<br>
#include "llvm/CodeGen/MachineFrameInfo.h"<br>
#include "llvm/CodeGen/MachineFunction.h"<br>
#include "llvm/CodeGen/MachineMemOperand.h"<br>
#include "llvm/CodeGen/PseudoSourceValue.h"<br>
-#include "llvm/IR/GlobalValue.h"<br>
#include "llvm/IR/ValueMap.h"<br>
#include "llvm/Target/TargetFrameLowering.h"<br>
#include "llvm/Target/TargetMachine.h"<br>
@@ -30,24 +28,6 @@<br>
<br>
namespace llvm {<br>
<br>
-/// \brief A class derived from PseudoSourceValue that represents a GOT entry<br>
-/// resolved by lazy-binding.<br>
-class MipsCallEntry : public PseudoSourceValue {<br>
-public:<br>
- explicit MipsCallEntry(StringRef N);<br>
- explicit MipsCallEntry(const GlobalValue *V);<br>
- bool isConstant(const MachineFrameInfo *) const override;<br>
- bool isAliased(const MachineFrameInfo *) const override;<br>
- bool mayAlias(const MachineFrameInfo *) const override;<br>
-<br>
-private:<br>
- void printCustom(raw_ostream &O) const override;<br>
-#ifndef NDEBUG<br>
- std::string Name;<br>
- const GlobalValue *Val;<br>
-#endif<br>
-};<br>
-<br>
/// MipsFunctionInfo - This class is derived from MachineFunction private<br>
/// Mips target-specific information for each MachineFunction.<br>
class MipsFunctionInfo : public MachineFunctionInfo {<br>
@@ -86,13 +66,13 @@ public:<br>
int getEhDataRegFI(unsigned Reg) const { return EhDataRegFI[Reg]; }<br>
bool isEhDataRegFI(int FI) const;<br>
<br>
- /// \brief Create a MachinePointerInfo that has a MipsCallEntr object<br>
- /// representing a GOT entry for an external function.<br>
- MachinePointerInfo callPtrInfo(StringRef Name);<br>
+ /// Create a MachinePointerInfo that has an ExternalSymbolPseudoSourceValue<br>
+ /// object representing a GOT entry for an external function.<br>
+ MachinePointerInfo callPtrInfo(const char *ES);<br>
<br>
- /// \brief Create a MachinePointerInfo that has a MipsCallEntr object<br>
+ /// Create a MachinePointerInfo that has a GlobalValuePseudoSourceValue object<br>
/// representing a GOT entry for a global function.<br>
- MachinePointerInfo callPtrInfo(const GlobalValue *Val);<br>
+ MachinePointerInfo callPtrInfo(const GlobalValue *GV);<br>
<br>
void setSaveS2() { SaveS2 = true; }<br>
bool hasSaveS2() const { return SaveS2; }<br>
@@ -142,11 +122,6 @@ private:<br>
/// FrameIndex for expanding BuildPairF64 nodes to spill and reload when the<br>
/// O32 FPXX ABI is enabled. -1 is used to denote invalid index.<br>
int MoveF64ViaSpillFI;<br>
-<br>
- /// MipsCallEntry maps.<br>
- StringMap<std::unique_ptr<const MipsCallEntry>> ExternalCallEntries;<br>
- ValueMap<const GlobalValue *, std::unique_ptr<const MipsCallEntry>><br>
- GlobalCallEntries;<br>
};<br>
<br>
} // end of namespace llvm<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">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>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">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>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">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>
</div></div></blockquote></div><br></div>