[PATCH] D35446: Add an ID field to StackObjects

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 19 16:55:36 PDT 2017


MatzeB added a reviewer: qcolombet.
MatzeB added a comment.

Seems fine to me, but needs more documentation and this touching a core regalloc datastructure I'm including Quentin in the reviewers.



================
Comment at: include/llvm/CodeGen/MachineFrameInfo.h:105-106
 
+    /// Identifier for stack analagous to address space.
+    uint8_t StackID;
+
----------------
This should have a more in-depth explanation. The notion that there is more than 1 stack will be very foreign to most people so there should be more than this sentence.


================
Comment at: include/llvm/CodeGen/MachineFrameInfo.h:607-614
+  uint8_t getStackID(int ObjectIdx) const {
+    return Objects[ObjectIdx+NumFixedObjects].StackID;
+  }
+  void setStackID(int ObjectIdx, uint8_t ID) {
+    assert(unsigned(ObjectIdx+NumFixedObjects) < Objects.size() &&
+           "Invalid Object Idx!");
+    Objects[ObjectIdx+NumFixedObjects].StackID = ID;
----------------
Should have some comments (could of course use something like \see StackID so you don't need to repeat the definition of what a StackID is).


https://reviews.llvm.org/D35446





More information about the llvm-commits mailing list