[clang] [clang][CodeGen][UBSan] Fixing shift-exponent generation for _BitInt (PR #80515)

Björn Pettersson via cfe-commits cfe-commits at lists.llvm.org
Sat Feb 3 02:51:59 PST 2024


================
@@ -4121,6 +4121,13 @@ Value *ScalarExprEmitter::GetWidthMinusOneValue(Value* LHS,Value* RHS) {
     Ty = cast<llvm::IntegerType>(VT->getElementType());
   else
     Ty = cast<llvm::IntegerType>(LHS->getType());
+  // Testing with small _BitInt types has shown that Ty->getBitwidth() - 1
----------------
bjope wrote:

Prefer if the code comment just help describe what it is doing, and possibly why here.
IMO, the "Testing with small _BitInt types has shown..." part is not really that interesting as a code comment.

One idea is to rename the `GetWidthMinusOneValue` helper function as we slightly change what it does.
It's more like a GetMaximumShiftAmount kind of helper. And there should be an update function description that describe a bit more about what is going on. I'm not sure why this static function is declared inside ScalarExprEmitter but the current description is far away at line 776.

It could say something like this:
Get a maximum legal shift exponent given a shift operation shifting the value LHS by RHS steps. The result type is given by RHS. The scalar bit width of the LHS value gives an upper bound on the shift exponent as it is considered illegal to shift more steps than "width minus one". If that value does not fit in the result type, then return an unsinged max for the result type.

https://github.com/llvm/llvm-project/pull/80515


More information about the cfe-commits mailing list