Page MenuHomePhabricator

Remove debug metadata from copied instruction to prevent GPUModule verification failure
ClosedPublic

Authored by singam-sanjay on Aug 1 2017, 10:11 AM.

Details

Summary

Remove debug metadata from instruction to be copied to prevent the source file's debug metadata being copied into GPUModule and eventually failing Module verification and ASM string codegeneration.

When copying the instruction onto the Module meant for the GPU, debug metadata attached to an instruction causes all related metadata to be pulled into the Module, including the DICompileUnit, which is not listed in llvm.dbg.cu of the Module. This fails the verification of the Module and generation of the ASM string.

The only debug metadata of the instruction, the DebugLoc, is unset by this patch.

This patch reattempts https://reviews.llvm.org/D35630 by targeting only those instructions that are to end up in a Module meant for the GPU.

Diff Detail

Repository
rL LLVM

Event Timeline

singam-sanjay created this revision.Aug 1 2017, 10:11 AM
grosser edited edge metadata.Aug 1 2017, 11:54 AM

Also, please update the summary to include the old commit message.

lib/CodeGen/BlockGenerators.cpp
249

Can you just check if Inst->getModule() != NewInst->getModule()

singam-sanjay edited the summary of this revision. (Show Details)

Simpler check to determine the destination Module of the cloned instruction.

grosser accepted this revision.Aug 1 2017, 1:05 PM

LGTM

This revision is now accepted and ready to land.Aug 1 2017, 1:05 PM
singam-sanjay marked an inline comment as done.Aug 1 2017, 9:11 PM
singam-sanjay added inline comments.
lib/CodeGen/BlockGenerators.cpp
249

Thanks for pointing this out ! I did think about this.

I had assumed that removing the DebugLoc from NewInst after inserting it into GPUModule wouldn't remove the Deubg metadata that came along with the instruction. But, moving the check before the insertion would have caused application to crash because NewInst isn't a part of a Module and NewInst->getModule() would seg fault. So, I went with an alternative approach.

grosser added inline comments.Aug 1 2017, 10:26 PM
lib/CodeGen/BlockGenerators.cpp
249

What is the alternative approach? Does the code as it is in phabricator work?

If NewInst->getModele does not work, you can always use Builder.GetInsertBlock()->getModule().

This revision was automatically updated to reflect the committed changes.
singam-sanjay marked an inline comment as done.
singam-sanjay added inline comments.Aug 2 2017, 8:21 AM
lib/CodeGen/BlockGenerators.cpp
249

The first diff was the alternative approach I was talking about. The code in phabricator does work.