[PATCH] XCore target: Add TypeString meta data to IR output.
Eric Christopher
echristo at gmail.com
Fri Apr 11 15:42:21 PDT 2014
Added a few more comments inline.
Thanks!
================
Comment at: lib/CodeGen/TargetInfo.cpp:6174
@@ -6117,1 +6173,3 @@
+void TypeStringCache::addIncomplete(const IdentifierInfo *ID,
+ std::string& Temp) {
----------------
Comment for the function.
================
Comment at: lib/CodeGen/TargetInfo.cpp:6178
@@ +6177,3 @@
+ return;
+ Entry& E = Map[ID]; // find Complete string or create an empty string.
+ Temp.swap(E.Str);
----------------
Sentences begin with a capital letter :)
================
Comment at: lib/CodeGen/TargetInfo.cpp:6184
@@ +6183,3 @@
+
+bool TypeStringCache::removeIncomplete(const IdentifierInfo *ID,
+ std::string& Temp) {
----------------
Comment for the function.
================
Comment at: lib/CodeGen/TargetInfo.cpp:6085
@@ +6084,3 @@
+ std::map<const IdentifierInfo *, struct Entry> Map;
+ unsigned IncompleteUsedCount;
+public:
----------------
What's going on with IncompleteUsedCount? I don't quite get it.
================
Comment at: lib/CodeGen/TargetInfo.cpp:6203
@@ +6202,3 @@
+
+bool TypeStringCache::add(const IdentifierInfo *ID, StringRef Str) {
+ if (!ID || IncompleteUsedCount)
----------------
Do you check the return value of this? Also comment.
================
Comment at: lib/CodeGen/TargetInfo.cpp:6213
@@ +6212,3 @@
+
+StringRef TypeStringCache::str(const IdentifierInfo *ID) {
+ if (!ID)
----------------
Comment on what's going on here. Why does "str" check the state?
================
Comment at: lib/CodeGen/TargetInfo.cpp:6318
@@ +6317,3 @@
+ IsComplete = TSC.removeIncomplete(ID, Temp);
+ if (!Success)
+ return false;
----------------
Why not check Success before the line above it?
================
Comment at: lib/CodeGen/TargetInfo.cpp:6320
@@ +6319,3 @@
+ return false;
+ if (isUnionType)
+ std::sort(FE.begin(), FE.end());
----------------
Sorting union encodings and not struct ones? Weird.
================
Comment at: lib/CodeGen/TargetInfo.cpp:6288
@@ +6287,3 @@
+ const CodeGen::CodeGenModule &CGM,
+ TypeStringCache &TSC, bool isUnionType,
+ const IdentifierInfo *ID) {
----------------
You can check whether the record is a union without passing it down as a bool.
http://reviews.llvm.org/D2929
More information about the llvm-commits
mailing list