[PATCH] D23355: getInstSizeInBytes: sentinel value fix for AArch64, AMDGPU, MSP430
Diana Picus via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 11 06:37:13 PDT 2016
rovka added inline comments.
================
Comment at: lib/Target/AArch64/AArch64BranchRelaxation.cpp:174
@@ +173,3 @@
+ // getInstSizeInBytes because here all instructions are seen for the first
+ // time.
+ if (InstSize == ~0U)
----------------
rovka wrote:
> SjoerdMeijer wrote:
> > rovka wrote:
> > > I'm not sure I understand this comment. Why wouldn't you check after every use of getInstSizeInBytes?
> > I wanted to avoid exactly that (checking it after every use) because this function is used quite a lot in this file.
> > This function computeBlockSize sees all instructions, so actually also checking it elsewhere would be redundant?
> Maybe it would now, but that might change someday. I think it would be safer to assert after every use.
> Anyway, if you insist on doing this, you should add a similar comment to MSP420BranchSelector, where you're also only checking after the first use.
430*
https://reviews.llvm.org/D23355
More information about the llvm-commits
mailing list