[PATCH] D48179: [AMDGPU] Emit MessagePack HSA Metadata for v3 code object

Nicolai Hähnle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 31 03:24:26 PDT 2018


nhaehnle added inline comments.


================
Comment at: include/llvm/Support/AMDGPUMetadata.h:455
+namespace Key {
+constexpr char Name[] = ".name";
+constexpr char TypeName[] = ".type_name";
----------------
This is admittedly a bit of bike-shedding, but what's the rationale for outlining the strings in this way?

As a general rule, I think it's better to have constant strings inline because it keeps the code base more easily discoverable. If I run into an ELF where something is wrong with `.name`, then inlined strings allow me to just grep for `".name"` and find all the relevant pieces of code immediately. With outlining, there's a level of indirection. I can find this location here easily, but then how do I reliably find the places where it's actually used? In this particular case, I can be reasonably confident that nobody is crazy enough to do `using namespace Key`, so a grep for `Key::Name` should do the trick, but it's still an extra step of indirection.


https://reviews.llvm.org/D48179





More information about the llvm-commits mailing list