[ARM][KFCI] Fix unused variable for #163698

[googlewalt]

No description provided.

[llvmbot]

@llvm/pr-subscribers-backend-arm

Author: Walter Lee (googlewalt)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/164857.diff

1 Files Affected:

• (modified) llvm/lib/Target/ARM/ARMAsmPrinter.cpp (+2)

diff --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 8e35cadb75857..958a595874b9c 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -1640,7 +1640,9 @@ void ARMAsmPrinter::EmitKCFI_CHECK_Thumb2(Register AddrReg, int64_t Type,
     bool isLast = (i == 3);
 
     // Verify the immediate can be encoded as Thumb2 modified immediate.
+#ifndef NDEBUG
     int T2SOImmVal = ARM_AM::getT2SOImmVal(imm);
+#endif // NDEBUG
     assert(T2SOImmVal != -1 &&
            "Cannot encode immediate as Thumb2 modified immediate");
 

[boomanaiden154]

See https://llvm.org/docs/CodingStandards.html#assert-liberally.

We normally prefer [[maybe_unused]] in these cases.

[googlewalt]

Is that the consensus? There are a dozen such usage in this directory already, and I have gotten push back for [[maybe_unused]] also.

[boomanaiden154]

Actually, looking at this case, the guidelines in the coding standards are to inline the call. So we just end up with:

    assert(ARM_AM::getT2SOImmVal(imm) != -1 &&
           "Cannot encode immediate as Thumb2 modified immediate");

This was changed recently (discourse post should be easy enough to find). But NDEBUG ifdefs I don't think were ever preferred in most areas of the repo.

[rnk]

I like this as-written. I don't think inlining the call into the assert harms the readability, and it's arguably more efficient, i.e. we don't do the encoding function call at all. It's not clear to me if the optimizer can remove that.