Philip Reames via llvm-dev
2019-Sep-27 16:28 UTC
[llvm-dev] What about multiple MachineMemOperands in one MI (BranchFolding/MachineInstr::mayAlias)?
On 9/27/19 7:33 AM, Matt Arsenault via llvm-dev wrote:> > >> On Sep 27, 2019, at 09:07, Björn Pettersson A via llvm-dev >> <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >> >> Obviously we do not store into two locations (it is still a single >> two byte store). >> So is it (always) correct to interpret the list of MachineMemOperands >> as the instruction will store to one of the locations? > > I think it’s bug to have multiple memory operands if the instruction > only accesses one location. The operands should have been merged in > some way unless the instruction can truly access two distinct addressesI'm a bit less sure of this. It's on the surface reasonable, but there are some interesting questions. We definitely interpret a list of MMOs as indicating a set of locations which are possibly(?) accessed. The only piece I'm unsure about is that the existence of an MMO requires the access occurs. If we do, that raises some interesting consistency questions for cases such as: * Load/Store merging (a superset of the branch folding case) * Predicate loads and stores (since the access may not happen) * Load/stores in dead code (i.e. the typical UB contradiction cases) * Instructions w/multiple accesses to the same MMO combined w/constant memory to imm folding which only handles some cases I'm tempted to suggest we treat the list of MMOs as a potential superset of the implied access, not a direct one-to-one mapping. (None of this should imply branch folding shouldn't merge the MMOs. That would just become an optimization quality issue, not a correctness one.) Philip> > -Matt > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190927/b022bc6c/attachment.html>
Amara Emerson via llvm-dev
2019-Oct-04 06:08 UTC
[llvm-dev] What about multiple MachineMemOperands in one MI (BranchFolding/MachineInstr::mayAlias)?
> On Sep 27, 2019, at 9:28 AM, Philip Reames via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > > > On 9/27/19 7:33 AM, Matt Arsenault via llvm-dev wrote: >> >> >>> On Sep 27, 2019, at 09:07, Björn Pettersson A via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >>> >>> Obviously we do not store into two locations (it is still a single two byte store). >>> So is it (always) correct to interpret the list of MachineMemOperands as the instruction will store to one of the locations? >> >> >> I think it’s bug to have multiple memory operands if the instruction only accesses one location. The operands should have been merged in some way unless the instruction can truly access two distinct addressesI would actually expect gather/scatter loads/stores to have multiple MMOs but according to SelectionDAG we generate just one, which seems technically incorrect.> I'm a bit less sure of this. It's on the surface reasonable, but there are some interesting questions. > > We definitely interpret a list of MMOs as indicating a set of locations which are possibly(?) accessed. The only piece I'm unsure about is that the existence of an MMO requires the access occurs. If we do, that raises some interesting consistency questions for cases such as: > > Load/Store merging (a superset of the branch folding case) > Predicate loads and stores (since the access may not happen) > Load/stores in dead code (i.e. the typical UB contradiction cases) > Instructions w/multiple accesses to the same MMO combined w/constant memory to imm folding which only handles some casesMaybe this should be specified properly somewhere, like a MIR langref. In GlobalISel we rely on MMOs being present and correct for legalization, which bakes in a 1-1 mapping assumption, at least for simple loads & stores.> I'm tempted to suggest we treat the list of MMOs as a potential superset of the implied access, not a direct one-to-one mapping. > > (None of this should imply branch folding shouldn't merge the MMOs. That would just become an optimization quality issue, not a correctness one.) > > Philip > > > >> >> -Matt >> >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev> > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191003/d84ea1fc/attachment.html>
Björn Pettersson A via llvm-dev
2019-Oct-04 14:02 UTC
[llvm-dev] What about multiple MachineMemOperands in one MI (BranchFolding/MachineInstr::mayAlias)?
Thanks for the answers! I’m thinking that maybe this should be described somewhere in MachineInstr.h. That is where I started to look for the information (since I wanted to know what it means when a MachineInstr has more than one MachineMemOperand). The comments related to the MachineInstr::memoperand() accessor says: /// Access to memory operands of the instruction. If there are none, that does /// not imply anything about whether the function accesses memory. Instead, /// the caller must behave conservatively. So it describes the case when there are none. But not what the existence of one or more operands implies. But maybe it should be mentioned in other places as well. I’ll be OOO for some days, I’ll see if I remember to get back to this later. (Main problem I had was related to the areMemAccessesTriviallyDisjoint target hook. I’ve realized that I should try to avoid looking at the mem operands inside that function.) Regards, Björn From: aemerson at apple.com <aemerson at apple.com> Sent: den 4 oktober 2019 08:09 To: Philip Reames <listmail at philipreames.com> Cc: Matt Arsenault <arsenm2 at gmail.com>; Björn Pettersson A <bjorn.a.pettersson at ericsson.com>; llvm-dev <llvm-dev at lists.llvm.org> Subject: Re: [llvm-dev] What about multiple MachineMemOperands in one MI (BranchFolding/MachineInstr::mayAlias)? On Sep 27, 2019, at 9:28 AM, Philip Reames via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote: On 9/27/19 7:33 AM, Matt Arsenault via llvm-dev wrote: On Sep 27, 2019, at 09:07, Björn Pettersson A via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote: Obviously we do not store into two locations (it is still a single two byte store). So is it (always) correct to interpret the list of MachineMemOperands as the instruction will store to one of the locations? I think it’s bug to have multiple memory operands if the instruction only accesses one location. The operands should have been merged in some way unless the instruction can truly access two distinct addresses I would actually expect gather/scatter loads/stores to have multiple MMOs but according to SelectionDAG we generate just one, which seems technically incorrect. I'm a bit less sure of this. It's on the surface reasonable, but there are some interesting questions. We definitely interpret a list of MMOs as indicating a set of locations which are possibly(?) accessed. The only piece I'm unsure about is that the existence of an MMO requires the access occurs. If we do, that raises some interesting consistency questions for cases such as: * Load/Store merging (a superset of the branch folding case) * Predicate loads and stores (since the access may not happen) * Load/stores in dead code (i.e. the typical UB contradiction cases) * Instructions w/multiple accesses to the same MMO combined w/constant memory to imm folding which only handles some cases Maybe this should be specified properly somewhere, like a MIR langref. In GlobalISel we rely on MMOs being present and correct for legalization, which bakes in a 1-1 mapping assumption, at least for simple loads & stores. I'm tempted to suggest we treat the list of MMOs as a potential superset of the implied access, not a direct one-to-one mapping. (None of this should imply branch folding shouldn't merge the MMOs. That would just become an optimization quality issue, not a correctness one.) Philip -Matt _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<https://protect2.fireeye.com/url?k=62f71123-3e7ecb0f-62f751b8-0cc47ad93da2-a0b0f919e0d6ac16&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-dev> _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<https://protect2.fireeye.com/url?k=ce256b71-92acb15d-ce252bea-0cc47ad93da2-996d109349ce3a32&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-dev> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191004/28e55e98/attachment.html>
Craig Topper via llvm-dev
2019-Oct-04 15:35 UTC
[llvm-dev] What about multiple MachineMemOperands in one MI (BranchFolding/MachineInstr::mayAlias)?
Gather is creating a single MMO using the "uniform base" with the scalar size of the gather. The indices aren't factored in. So I don't think its even pointing to the right place. ~Craig On Thu, Oct 3, 2019 at 11:08 PM Amara Emerson via llvm-dev < llvm-dev at lists.llvm.org> wrote:> > > On Sep 27, 2019, at 9:28 AM, Philip Reames via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > > On 9/27/19 7:33 AM, Matt Arsenault via llvm-dev wrote: > > > > On Sep 27, 2019, at 09:07, Björn Pettersson A via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > Obviously we do not store into two locations (it is still a single two > byte store). > So is it (always) correct to interpret the list of MachineMemOperands as > the instruction will store to one of the locations? > > > I think it’s bug to have multiple memory operands if the instruction only > accesses one location. The operands should have been merged in some way > unless the instruction can truly access two distinct addresses > > I would actually expect gather/scatter loads/stores to have multiple MMOs > but according to SelectionDAG we generate just one, which seems technically > incorrect. > > I'm a bit less sure of this. It's on the surface reasonable, but there > are some interesting questions. > > We definitely interpret a list of MMOs as indicating a set of locations > which are possibly(?) accessed. The only piece I'm unsure about is that > the existence of an MMO requires the access occurs. If we do, that raises > some interesting consistency questions for cases such as: > > - Load/Store merging (a superset of the branch folding case) > - Predicate loads and stores (since the access may not happen) > - Load/stores in dead code (i.e. the typical UB contradiction cases) > - Instructions w/multiple accesses to the same MMO combined w/constant > memory to imm folding which only handles some cases > > Maybe this should be specified properly somewhere, like a MIR langref. In > GlobalISel we rely on MMOs being present and correct for legalization, > which bakes in a 1-1 mapping assumption, at least for simple loads & stores. > > > > I'm tempted to suggest we treat the list of MMOs as a potential superset > of the implied access, not a direct one-to-one mapping. > > (None of this should imply branch folding shouldn't merge the MMOs. That > would just become an optimization quality issue, not a correctness one.) > > Philip > > > > -Matt > > _______________________________________________ > LLVM Developers mailing listllvm-dev at lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191004/7a17da21/attachment.html>