[PATCH] D57718: [PPC] Adjust the computed branch offset for the possible shorter distance

[Jinsong Ji via Phabricator via llvm-commits]

jsji added inline comments.


================
Comment at: lib/Target/PowerPC/PPCBranchSelector.cpp:214
+          //    ...
+          //   bne Far      100            10c
+          //   .p2align 4
----------------
Carrot wrote:
> jsji wrote:
> > Looks like to me that the root cause here is that 
> > 1. we **may have conservative (larger) MBBStartOffset** due to inline asm or alignment,  (eg: `100` -> `10c`)
> > 2.  we are **subtracting MBBStartOffset** here  (eg: `0x8108 - 0x10c`)
> > 
> > So instead of getting larger `BranchSize`, subtracting may lead us to getting smaller `BranchSize`.
> > 
> > For other blocks, even if we may still have conservative (larger) size, it should be OK, as we are adding them towards `BranchSize`.
> > 
> > 
> > So, shouldn't we just need to adjust the value of `MBBStartOffset` , checking inline asm & alignment when calculating it in previous loop?
> The root cause in your understanding is correct!
> 
> The inline asm size is handled by 
>   MBBStartOffset += TII->getInstSizeInBytes(*I);
> The BB alignment is already handled when computing BlockSizes.
> 
> Here the problem is the computed MBBStartOffset can't be precise, and we can't tell what's the difference between its value and the actual address. For the BranchSize, if it is larger than actual offset, it is safe for us. If it is smaller than actual branch offset, the max delta is 
>       (1 << MaxAlign) - 4
> 
Thanks. 
Can we tell whether `MBBStartOffset` is precise or not by checking whether we meet inline asm & alignment?
And only apply the delta when we are sure they are not precise?

Also is it possible that `MBBStartOffset` is smaller than ` (1 << MaxAlign) -4 `?

And can we add some comments about why the max delta is 
` (1 << MaxAlign) -4 ` ?




Repository:
  rL LLVM

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

https://reviews.llvm.org/D57718