[llvm-dev] [SelectionDAG] Assertion due to MachineMemOperand flags difference.

[Sean Fertile via llvm-dev]

Hi Hal,

Thanks for the reply, that clears up a fair bit for me. I think the
reason we have the 'FixedStack'  location without it being marked as
dereferenceable is because its created in CreateCopyOfByValArgument
where we use a default MachinePointerInfo in the call to getMemcpy. As
for fixing the assertion I think I understand now why we look up a
node with different MMO flags, its a 'MemIntrinsicSDNode' which
doesn't add its subclass data to the FoldingSetNode. I'll update the
patch on D38898 accordingly.

Thanks
Sean


On Wed, Oct 25, 2017 at 8:28 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> On 10/13/2017 06:05 PM, Sean Fertile via llvm-dev wrote:
>
> Hello,
>
> I've hit an assertion in SelectionDAG where we try to merge 2 loads
> that have the same operands but their MMO flags differ. One is
> dereferenceable and one is not. I'm not sure what the underlying issue
> here is:
>
> 1) MDSDNode with the same operands should have the same flags set on
> their respective MMO. The fact the flags differ when the
> opcode,types,operands and address-space are the same is the problem.
>
> 2) Having MDSDnodes with the same operands but different MMO flags is
> possible, so the Flags should be added to the FoldingSetNodeID.
>
> 3) Something else I haven't considered.
>
> I have a patch posted implementing 2, but don't know if I should look
> at fixing 1 as well (or perhaps instead). The loads that trigger the
> assertion are:
>
> t47: v4i32,ch = load<LD16[%0+80](align=8)(dereferenceable)> t20, t46,
> undef:i64
> t69: v4i32,ch = load<LD16[FixedStack1+80](align=8)> t50, t46, undef:i64
>
> I would expect the the second load should also be marked
> dereferenceable since its loading from one of the TargetFrames. Am I
> on the right track here?
>
>
> Hi, Sean,
>
> To follow-up on this, first I'll note that, IIRC, we've fixed a couple of
> related problems in the recent past. Specifically, the following come to
> mind: r309930 and r306404. In this case, it seems like something is dropping
> this information somewhere. Or, perhaps, I don't see why we have a
> "FixedStack" location that's not marked as dereferenceable. That having been
> said...
>
> As for whether "having MDSDnodes with the same operands but different MMO
> flags is possible", the answer is yes. Of the bits that are currently
> defined, MOLoad and MOStore are attached to fundamental properties of the
> corresponding SDNode. Of the remainder: MOVolatile, MONonTemporal,
> MODereferenceable, and MOInvariant, some of these can be set independently
> on a per-operation basis. For example, one can certainly have a two volatile
> loads of the same address, one which is non-temporal and one which is not.
> However, those bits are already added to the FoldingSetNodeID, because
> they're mirrored by the MemSDNode's MemSDNodeBits (which has bits for
> IsVolatile, IsNonTemporal, IsDereferenceable, and IsInvariant), and these
> should be returned by getRawSubclassData, which is called in all of the
> places that D38898 modifies). Thus, in short, I don't understand why the
> IsDereferenceable bit is not already included in the FoldingSetNodeID.
>
>  -Hal
>
>
> Thanks
> Sean
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory