Allow naming of llvm symbols for many ops #1530
Conversation
Most LLVM API IR calls that generate ops allow an optional name of the llvm symbol that will hold the result of the operation. By default, it will make one up, such as "%37", "%38", etc. Needless to say, these are difficult to understand when reading volumes of IR dumped for debugging purposes. So in this patch, I'm starting to expose the ability for our own code to pass names through our wrapper functions. I needed this for some deep debugging. I haven't instrumented the entire set, just trying it on for some of the specific areas I needed to debug. But this is a useful first step. Signed-off-by: Larry Gritz <[email protected]>
There was a problem hiding this comment.
Overall LGTM. I am assuming that we'll still get some postfix of the _## variety to keep the single-static-assignment properties, is that correct? If not that would be problematic in any but the simplest cases and we'd need to add one ourselves.
Also, I wouldn't mind seeing this on the wide ops as well. I'm glad you started this...it should be immensely valuable in identifying missed optimization opportunities and debugging IR gen.
| @@ -590,7 +591,7 @@ BackendLLVM::llvm_load_value(llvm::Value* ptr, const TypeSpec& type, int deriv, | |||
| ptr = ll.GEP(ptr, 0, component); | |||
|
|
|||
| // Now grab the value | |||
| llvm::Value* result = ll.op_load(ptr); | |||
| llvm::Value* result = ll.op_load(ptr, llname); | |||
|
|
|||
There was a problem hiding this comment.
For these more complicated entry points, I'm wondering if it would be more useful to bake things like deriv, arrayindex, component into the name so they are there in a consistent way. Also, naming the intermediate results, such as the offset here so they are easily tied together as optimizations shuffle them around may also be helpful.
There was a problem hiding this comment.
LLVM automatically appends an incrementing counter to the string, if it is not unique.
How these are used will be a little more clear in my next PR, which is an overhaul of how strings are handled on GPU (and makes use of this). This is just setting up some of the debugging infrastructure that looked like I could cleave it off into a separate self-contained PR. I've just scratched the surface, I think this will continue to evolve as we realize what information is useful to embed in the names.
There was a problem hiding this comment.
I've spent a lot of time in the last several days looking at IR line by line to understand what it's doing wrong. :-) It forced a whole digression into leaving more breadcrumbs in the IR so I could understand it.
There was a problem hiding this comment.
@sfriedmapixar I definitely will extend to wide ops and maybe universally, but not as part of this first PR. I need to put the string stuff to bed first.
There was a problem hiding this comment.
Sounds completely resonable.
…1530) Most LLVM API IR calls that generate ops allow an optional name of the llvm symbol that will hold the result of the operation. By default, it will make one up, such as "%37", "%38", etc. Needless to say, these are difficult to understand when reading volumes of IR dumped for debugging purposes. So in this patch, I'm starting to expose the ability for our own code to pass names through our wrapper functions. I needed this for some deep debugging. I haven't instrumented the entire set, just trying it on for some of the specific areas I needed to debug. But this is a useful first step. Signed-off-by: Larry Gritz <[email protected]>
Most LLVM API IR calls that generate ops allow an optional name of the
llvm symbol that will hold the result of the operation. By default, it
will make one up, such as "%37", "%38", etc. Needless to say, these
are difficult to understand when reading volumes of IR dumped for
debugging purposes.
So in this patch, I'm starting to expose the ability for our own code
to pass names through our wrapper functions. I needed this for some
deep debugging.
I haven't instrumented the entire set, just trying it on for some of
the specific areas I needed to debug. But this is a useful first step.
Signed-off-by: Larry Gritz [email protected]