[PATCH] D28150: Move the section name from GlobalObject to the LLVMContext

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini accepted this revision.
mehdi_amini added a reviewer: mehdi_amini.
mehdi_amini added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: include/llvm/IR/GlobalObject.h:41
-  std::string Section;     // Section to emit this into, empty means default
   Comdat *ObjComdat;
   enum {
----------------
We could do the same for the comdat?


================
Comment at: include/llvm/IR/GlobalObject.h:75
+  }
+  StringRef getSectionImpl() const;
   void setSection(StringRef S);
----------------
Private?


================
Comment at: lib/IR/Globals.cpp:188
+  setGlobalValueSubClassData((~Mask & getGlobalValueSubClassData()) |
+                             (HasSection ? Mask : 0u));
 }
----------------
I'd write it along these lines:

```
unsigned Mask = 1 << HasSectionHashEntryBit;
auto Bitfield = getGlobalValueSubClassData();
if (HasSection) 
  // Set the `HasSectionHashEntryBit`
  Bitfield |= Mask;
else
  // Clear the `HasSectionHashEntryBit`
  Bitfield &= ~Mask;
setGlobalValueSubClassData(Bitfield);
```

Seems less complex to read to me than the `|` with a ternary on the RHS that may lead to a 0.

Otherwise extracting it in a `setGlobalValueSubClassDataBitValue(unsigned BitNo, bool Value)` would make the implementation of it trivial.


https://reviews.llvm.org/D28150