[PATCH] D130316: [SelectionDAG] make INLINEASM_BR use MachineBasicBlocks instead of BlockAddresses

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5294-5295
+      OpInfo.CallOperandVal = BA->getBasicBlock();
       OpInfo.ConstraintVT =
-          getAsmOperandValueType(DL, OpInfo.CallOperandVal->getType())
-              .getSimpleVT();
+        getAsmOperandValueType(DL, BA->getType()).getSimpleVT();
       ++LabelNo;
----------------
nikic wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > nickdesaulniers wrote:
> > > > > nikic wrote:
> > > > > > nickdesaulniers wrote:
> > > > > > > Not sure this is quite right, but also not sure what it should be.  Maybe the reviewers do?
> > > > > > `BA` has an `i8*` type, so can directly use that?
> > > > > `Type::getInt8PtrTy` seems to cause crashes...cant tell what type is actually being used; everything looks like opaque ptrs...
> > > > branch-folder is messing something up...
> > > ahoy! Both TailDuplicator and BranchFolding were relying on whether an MBB has its address taken. After this change, they wont, so those cases additionally need to check for isInlineAsmBrIndirectTarget. With that added, we're working AND the .Ltmp labels are gone (good)!  Now just to clean up the test churn after lunch, but I think this will all work and look nice. Stay tuned.
> > The last wart on an otherwise great simplification is getting the context for `Type::getInt8PtrTy`. Thoughts on how I can improve this?
> I think you can just use `getProgramPointerTy(DL)` to directly get the right MVT. No need to indirect via IR types.
> 
> You should also be able to drop the getBlockAddressForIndirectDest() method.
getProgramPointerTy is correct but I would expect taking the type directly from the IR would be better (in principle we could have individual functions with a different address space from the DL one, although I think that's currently unsupported)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130316