[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