yaohuihan-iluvatar wrote:

> I don't know anything about auxiliary builtin IDs, what they are or why they 
> exist. BUT:
> 
> 1. If this is so important, shouldn't `getConstantEvaluatedBuiltinID` be a 
> member function of `CallExpr`?
> 2. `CallExpr::getBuiltinID()` is already very slow, so adding another layer 
> do that seems bad.

Thanks for your review.

For Q1:

- It's a good idea that making `getConstantEvaluatedBuiltinID` be a member 
function of `CallExpr`. The aux-ID normalization (`isAuxBuiltinID → 
getAuxBuiltinID`) is duplicated in several places — `CGBuiltin.cpp 
(EmitTargetBuiltinExpr)`, `CIRGenBuiltin.cpp`, `SemaChecking.cpp`, 
`SemaARM.cpp`, `Builtins.cpp` — so centralizing it makes sense.

- However, this requires the maintainer to confirm that the bug needs to be 
fixed.

For Q2:

- On performance: `getConstantEvaluatedBuiltinID` calls `getBuiltinCallee()` 
exactly once; everything after that is just `isAuxBuiltinID` (a comparison) and 
`getAuxBuiltinID` (a subtraction), both O(1). It does not add a second call to 
the slow `FunctionDecl::getBuiltinID()` — it's a thin wrapper around a single 
call, not an extra layer stacked on top of it.

https://github.com/llvm/llvm-project/pull/201805
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to