Follow up to https://reviews.llvm.org/D145238 that enforces native half type support is present when using builtins and gives more descriptive error in not.
Tidy up CodeGenFunction::EmitNVPTXBuiltinExpr by moving helper lambdas to dedicated functions (most of helpers already reside in dedicated functions).
Details
- Reviewers
tra - Commits
- rGae3c981aa4b8: [NVPTX] Enforce half type support is present for builtins
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/lib/CodeGen/CGBuiltin.cpp | ||
---|---|---|
18772 | We seem to be checking builtin IDs twice. Once here and then in MakeHalfType where we need to map them to intrinsics. Another option you may consider is that if the availability is only dependent on PTX or SM version, then we may get by with declaring the builtins as TARGET_BUILTIN with appropriate constraints and let the standard enforcement machinery to handle diags. |
clang/lib/CodeGen/CGBuiltin.cpp | ||
---|---|---|
18772 | I've refactored those switches and passed intrinsic ID to MakeHalfType helper. |
clang/lib/CodeGen/CGBuiltin.cpp | ||
---|---|---|
18331 | This is where lambda would have some advantage as we could capture what it needs without having to pass 'this' or 'E' explicitly. |
clang/lib/CodeGen/CGBuiltin.cpp | ||
---|---|---|
18331 | My reasoning was that it keeps CodeGenFunction::EmitNVPTXBuiltinExpr clean, all that it does now is to dispatch to a correct handler based on the builtin id (with exception of mmas); you are right though, it does it at the price of making the signatures more verbose. |
This is where lambda would have some advantage as we could capture what it needs without having to pass 'this' or 'E' explicitly.
In this case it's not too bad, so I'm fine either way, with a very slight bias towards lambdas. The static functions just don't seem to buy us anything here, IMO.