[PATCH] D132802: [SPIR-V] Use std::optional for builtin lowering result.

Ilia Diachkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Aug 28 15:28:56 PDT 2022


iliya-diyachkov added a comment.

@zuban32, perhaps it's better to use `llvm::Optional` from llvm/ADT/Optional.h (see https://github.com/KhronosGroup/LLVM-SPIRV-Backend/pull/217/files). It's widely used in llvm. On other hand, `std::optional` is not used at all in llvm/lib (as I see in llvm15).



================
Comment at: llvm/lib/Target/SPIRV/SPIRVBuiltins.h:25
 ///
 /// \return a pair of boolean values, the first true means the call recognized
 /// as a builtin, the second one indicates the successful lowering.
----------------
We need to correct the description too, e.g.
```
/// \return A boolean value indicating the successful lowering, if the callee
/// is recognized as a builtin function.
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132802/new/

https://reviews.llvm.org/D132802



More information about the llvm-commits mailing list