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
