Enable write to global vars with HLSLVersion <= 2016 - #1526
Conversation
Tex Riddell (tex3d)
left a comment
Tex Riddell (tex3d)
left a comment
There was a problem hiding this comment.
Let's change this to add /Gec option, and:
- Default HLSLVersion to 2016 while emitting a warning that this is deprecated functionality.
- Emit error if /HV is used to explicitly set version greater than 2016 (2015 is ok for non-codegen path).
- Change the code in Parser::ParseDeclGroup that sets the global decl to const for HLSL to avoid this when compat flag is set.
- Revert changes to SemaExpr.cpp.
|
Updated. |
| } | ||
|
|
||
| static GlobalVariable *CreateStaticGlobal(llvm::Module *M, GlobalVariable *GV) { | ||
| Constant *GC = M->getOrInsertGlobal(GV->getName().str() + "_static_copy", |
There was a problem hiding this comment.
You should use name suffix that could not be an HLSL identifier name, such as ".static.copy", since that's valid for llvm, and guaranteed to avoid collision with any HLSL name defined.
There was a problem hiding this comment.
fixed
| static bool CreateWriteEnabledStaticGlobals(llvm::Module *M) { | ||
| std::vector<GlobalVariable *> worklist; | ||
| for (GlobalVariable &GV : M->globals()) { | ||
| if (!GV.isConstant()) { |
There was a problem hiding this comment.
Also skip GVs that are InternalLinkage to skip actual user static globals.
There was a problem hiding this comment.
fixed
| } | ||
|
|
||
| GlobalVariable *NGV = CreateStaticGlobal(M, GV); | ||
| // insert memcpy in all entryblocks |
There was a problem hiding this comment.
You only want to insert memcpy into entry block of main EntryFunction, not all functions.
There was a problem hiding this comment.
fixed
| for (User *U : GV->users()) { | ||
| if (Instruction *I = dyn_cast<Instruction>(U)) { | ||
| Function *F = I->getParent()->getParent(); | ||
| if (F->getName() == "main") { |
There was a problem hiding this comment.
Main entry function is not necessarily called "main". You should pass the function in to CreateStaticGlobal() from FinishCodeGen. You can get the main function by calling GetEntryFunction() on the HLModule. And then you don't need to search for users that are in this function either, since there may be no users in main function, but you still want to insert memcpy there.
| for (User *U : V->users()) { | ||
| if (isa<StoreInst>(U)) | ||
| isWriteEnabled = true; | ||
| else if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(U)) { |
There was a problem hiding this comment.
This can be a constant GEP expression, so best to use GEPOperator. Further, you can have multiple levels of GEP at this point, so best to recurse.
| isWriteEnabled = true; | ||
| else | ||
| isWriteEnabled |= IsWriteEnabledGlobalRec(GU, visited); | ||
| } else isWriteEnabled |= IsWriteEnabledGlobalRec(U, visited); |
There was a problem hiding this comment.
You should break as soon as isWriteEnabled == true, since you don't need to search anymore. In fact, this means you don't need the local variable, since you just return true when you find a store, or when a recursive call returns true. Then return false at the end, when you didn't find a store.
| // Returns true if any of the direct user of a global is a store inst | ||
| // otherwise recurse through the remaining users and check if any GEP | ||
| // exists and which in turn has a store inst as user. | ||
| static bool IsWriteEnabledGlobal(GlobalVariable *GV) { |
There was a problem hiding this comment.
Perhaps a better name would be GlobalHasStoreUser.
| } | ||
| case HLOpcodeGroup::HLCast: | ||
| case HLOpcodeGroup::HLSubscript: | ||
| isWriteEnabled |= GlobalHasStoreUserRec(U, visited); |
There was a problem hiding this comment.
Replace:
isWriteEnabled |= GlobalHasStoreUserRec(U, visited);
with:
if (GlobalHasStoreUserRec(U, visited))
return true;
and return false at the end, then you can get rid of the local isWriteEnabled variable.
3 participants
Enable write to global vars with HLSLVersion <= 2016