[llvm] [llvm/llvm-project][Coroutines] Improve debugging and minor refactoring (PR #104642)

Yuxuan Chen via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 27 12:02:53 PDT 2024


================
@@ -418,10 +440,7 @@ struct RematGraph {
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   void dump() const {
     dbgs() << "Entry (";
-    if (EntryNode->Node->getParent()->hasName())
-      dbgs() << EntryNode->Node->getParent()->getName();
-    else
-      EntryNode->Node->getParent()->printAsOperand(dbgs(), false);
+    dbgs() << getBasicBlockLabel(EntryNode->Node->getParent());
----------------
yuxuanchen1997 wrote:

My only concern was the unnecessary copy of basic block name. You can do something with a macro like 
```
#define BB_NAME(BB) (BB->hasName() ? BB->getName() : StringRef(getBasicBlockLabel(BB)))
```
This uses lifetime extension rules to make the latter remain allocated while used as a temporary. 
But this is not just ugly, but also unsafe to be stored to a variable. A better way to do this is through a type similar to a tagged union of `StringRef` and `std::string`. 

There is probably no point in optimizing this for a single dump method. With these reasons, I am not against landing this now. But since you mentioned that RematGraph is also using similar patterns, there might be motivation to solve it systematically. (e.g. `BasicBlock::getDebugName()`)

https://github.com/llvm/llvm-project/pull/104642


More information about the llvm-commits mailing list