[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