Ignoring construct field warnings on delegatory methods #4911
Conversation
| for (auto use = inst->firstUse; use; use = use->nextUse) | ||
| { | ||
| IRInst* user = use->getUser(); | ||
| collectLoadStore(stores, loads, user, inst); |
There was a problem hiding this comment.
This seems very inefficient. All we care about here is whether there are any stores, we don't want to collect all load and stores just to do the check.
There was a problem hiding this comment.
I've reworked the collection system so that now there are methods to check the usage type of an instruction wrt an operand. With this we can skip the collection and do a pure check.
venkataram-nv
force-pushed
the
softed-constructor-warnings
branch
from
25db183 to
1744146
Compare
| Load // Instruciton acts as a load from the source | ||
| }; | ||
|
|
||
| static InstructionUsageType getCallUsageType(IRCall* call, IRInst* inst) |
There was a problem hiding this comment.
The logic inside this function for handling backward differentiate isn't correct. The inner function's type is usually not the same as the function type of the differentiated function. I suggest we don't check for any autodiff cases here and just assume that if we can't obtain the func type, then whenever a pointer typed argument is passed in a call, it is inout.
There was a problem hiding this comment.
If you are not sure if certain logic is correct, make sure to call it out in the comment or the PR description so we don't get incorrect logic slipped through the review process.
There was a problem hiding this comment.
The diagnostics currently ignore functions marked as differentiable, so this shouldn't be an issue:
// for example in checkUninitializedValues
if (isDifferentiableFunc(func))
return;The reason they are ignored is because at the moment it is not possible to distinguish functions marked as differentiable to be something that the user wrote or that slang synthesized automatically.
There was a problem hiding this comment.
If Slang is synthesizing code that leads to warnings, we should fix those synthesis logic. Nevertheless, the logic here is incorrect and we should remove them and replace with an SLANG_UNEXPECTED failure.
venkataram-nv
force-pushed
the
softed-constructor-warnings
branch
from
cae410f to
985b64f
Compare
venkataram-nv
force-pushed
the
softed-constructor-warnings
branch
from
985b64f to
01e8398
Compare
Fixes #4872