[PATCH] D64982: [AsmPrinter] Print label if MBB's address is taken

Bill Wendling via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 19 03:39:09 PDT 2019


void marked an inline comment as done.
void added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp:438
                   MBB.getSymbol()->print(OS, AP->MAI);
+                  const_cast<MachineBasicBlock*>(&MBB)->setLabelMustBeEmitted();
                   break;
----------------
nickdesaulniers wrote:
> void wrote:
> > nickdesaulniers wrote:
> > > Make `MBB` a non-const `MachineBasicBlock&`, then you can drop the ugly `const_cast` and then also the `&` then `->` and just use `.`.
> > > 
> > > ```
> > > for (MachineBasicBlock& MBB : *MF) {
> > >   ...
> > >   MBB.setLabelMustBeEmitted();
> > > ```
> > The `MF` is coming from `MI` which itself is `const`. There's probably going to be a `const_cast` somewhere along the line. :-(
> Ok, is the `&`+`->` still really necessary?
> ```
> const_cast<MachineBasicBlock>(MBB).setLabelMustBeEmitted();
> ```
> 
> (I still feel like `const_cast` is a code smell; if we modify something, then our function signature should really not accept a `const` param.  `const` parameters are meant to be an agreement between caller and callee that callee doesn't modify the param).
I can cast it to a reference.

I don't like `const_cast`s either. We would need to calculate that it should be set long before getting here.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64982/new/

https://reviews.llvm.org/D64982





More information about the llvm-commits mailing list