[PATCH] D40808: [RISCV] Implement branch analysis

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/RISCVInstrInfo.cpp:152
+
+  // Get the last instruction in the block.
+  MachineInstr *LastInst = &*I;
----------------
reames wrote:
> Okay, this code structure is horrible.  I get that you just copied the structure, but there's no reason to keep this.  Suggested alternate structure:
> if (!ends with predicated terminator) // i.e. fallthrough
>   return
> scan back to first unconditional branch or indirect branch
> if (allowmodify) {
>   // remove everything after that point
> }
> handle conditional branch case starting from last unconditional branch
> handle unconditional branch case starting from last unconditional branch
> 
> Beyond this, there's a general invariant question.  How do we end up with multiple unconditional branches?  When we insert one, shouldn't we truncate the block at that point?
I agree that truncating upon insertion might make sense, but I think we need to work within the constraints of the LLVM interfaces for this patch. analyzeBranch is free to modify a BB and it's not clear what would break if insertBranch did the same.


https://reviews.llvm.org/D40808