[llvm-dev] Ok with mismatch between dead-markings in BUNDLE and bundled instructions?
Mikael Holmén via llvm-dev
llvm-dev at lists.llvm.org
Thu Jul 6 06:41:39 PDT 2017
On 07/05/2017 07:02 PM, Quentin Colombet wrote:
>
>> On Jul 3, 2017, at 4:13 AM, Mikael Holmén <mikael.holmen at ericsson.com> wrote:
>>
>> Hi,
>>
>> Ok, so to solve the problem for our out-of-tree target we currently do:
>>
>> ------------------------ lib/CodeGen/InlineSpiller.cpp ------------------------
>> index 67c7e506add..28245a49477 100644
>> @@ -987,38 +987,46 @@ void InlineSpiller::spillAroundUses(unsigned Reg) {
>> if (foldMemoryOperand(Ops))
>> continue;
>>
>> // Create a new virtual register for spill/fill.
>> // FIXME: Infer regclass from instruction alone.
>> unsigned NewVReg = Edit->createFrom(Reg);
>>
>> if (RI.Reads)
>> insertReload(NewVReg, Idx, MI);
>>
>> // Rewrite instruction operands.
>> bool hasLiveDef = false;
>> for (const auto &OpPair : Ops) {
>> MachineOperand &MO = OpPair.first->getOperand(OpPair.second);
>> MO.setReg(NewVReg);
>> if (MO.isUse()) {
>> if (!OpPair.first->isRegTiedToDefOperand(OpPair.second))
>> MO.setIsKill();
>> } else {
>> + // For bundled instructions: Only examine defs in the BUNDLE
>> + // instruction itself if it exists.
>> + MachineInstr *DefMI = OpPair.first;
>> + if (DefMI->isBundled() &&
>> + getBundleStart(DefMI->getIterator())->isBundle() &&
>> + !DefMI->isBundle())
>> + continue;
>> +
>> if (!MO.isDead())
>> hasLiveDef = true;
>> }
>> }
>> DEBUG(dbgs() << "\trewrite: " << Idx << '\t' << *MI << '\n');
>>
>>
>> Is there any point of me putting this patch in Phabricator? I have very little hope of managing to trigger the problem on any in-tree target.
>
> You could use a .mir file, with NOOPs that clobber a lot of regs and fake bundles here and there.
>
Yes, it was much easier than I thought it would be!
With one eye on test/CodeGen/MIR/AArch64/spill-fold.mir I managed to
create a test reproducing the issue on aarch64.
Patch here:
https://reviews.llvm.org/D35055
Thanks,
Mikael
>
>>
>> Regards,
>> Mikael
>>
>>
>> On 06/29/2017 02:13 AM, Quentin Colombet via llvm-dev wrote:
>>>> On Jun 28, 2017, at 5:10 PM, Quentin Colombet via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>>>>
>>>> Oh wait, vreg1 is indeed used.
>>>> Yeah, having a dead flag here sounds wrong.
>>> I mean on the instruction itself.
>>> On the bundle, that’s debatable. That would fit the semantic “if no side effect you can kill it” (here there is side effect, we define other vregs).
>>>>
>>>>> On Jun 28, 2017, at 2:28 AM, Björn Pettersson A <bjorn.a.pettersson at ericsson.com> wrote:
>>>>>
>>>>> Not sure if I could follow everything in this discussion regarding subregisters. But I think the problem posted by Mikael just happened to involve subregisters, and the discussions about subregisters is confusing when it comes to Mikaels original question/problem.
>>>>>
>>>>> I think that the bundle could look something like this just as well:
>>>>>
>>>>> BUNDLE %vreg1<def,dead>
>>>>> * %vreg1<def> = add %vreg2, %vreg3
>>>>> * call @foo, %vreg1<internal-use>
>>>>>
>>>>> No subregisters involved.
>>>>> %vreg1 is dead after the bundle.
>>>>> %vreg1 is not dead when defined at the "add", because it is used later in the same bundle.
>>>>>
>>>>> Should perhaps the %vreg1 not be included in the BUNDLE head at all here?
>>>>> (but shouldn't the BUNDLE head be a summary of what is going on inside the bundle, so leaving out information about %vreg1 being defined seems wrong)
>>>>>
>>>>> To me it seems wrong to add "dead" to the def of %vreg1 at the add (considering the internal-use).
>>>>> Maybe that even answers the question that the "mismatch" between dead-markings should be OK.
>>>>> Or would it be OK to mark %vreg1 as dead at the add, even though we have a later internal-use?
>>>>>
>>>>> Regards,
>>>>> Björn
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
>>>>>> Quentin Colombet via llvm-dev
>>>>>> Sent: den 28 juni 2017 00:02
>>>>>> To: Matthias Braun <mbraun at apple.com>
>>>>>> Cc: llvm-dev at lists.llvm.org
>>>>>> Subject: Re: [llvm-dev] Ok with mismatch between dead-markings in
>>>>>> BUNDLE and bundled instructions?
>>>>>>
>>>>>>
>>>>>>> On Jun 27, 2017, at 2:51 PM, Matthias Braun via llvm-dev <llvm-
>>>>>> dev at lists.llvm.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>>> On Jun 27, 2017, at 2:44 PM, Krzysztof Parzyszek via llvm-dev <llvm-
>>>>>> dev at lists.llvm.org> wrote:
>>>>>>>>
>>>>>>>> On 6/27/2017 4:35 PM, Quentin Colombet via llvm-dev wrote:
>>>>>>>>> Yeah I was reading this as “only the non-touched part are dead”, and
>>>>>> that’s what I’d like to see in the representation longer. Obviously, the
>>>>>> register is not dead as a whole here :)
>>>>>>>>
>>>>>>>> I think that having two defs for the same register, one dead and one not
>>>>>> dead simply doesn't make sense. We already assume that a register is live if
>>>>>> at least a part of it is live, so if it's "dead", it should mean that the whole thing
>>>>>> is dead.
>>>>>>> Without subregister I would agree. However with subregisters and aliases
>>>>>> in play you can express more situations. Like for example:
>>>>>>>
>>>>>>> %rax<dead>, %eax = ...
>>>>>>>
>>>>>>> could mean the instruction writes the full rax register but we are only
>>>>>> gonna read eax later.
>>>>>>
>>>>>> That sounds like an alias to:
>>>>>> %rax<def-undef, subeax> = …
>>>>>>
>>>>>> Which sounds fine. Though I am not suggesting we want to move to this
>>>>>> dead model for such situation.
>>>>>>
>>>>>>> That said I am not sure whether we actually need it, and if llvm works that
>>>>>> way today. Given how subtle all of this is there is also a high danger that we
>>>>>> won't get the bahviour consistent.
>>>>>>
>>>>>> I agree that consistent behavior is important and I also think we probably
>>>>>> cannot model what we want with the current representation. What I would
>>>>>> like to see if that we don’t sit on potentially useful information, like this part
>>>>>> of the register is dead, because it is convenient implementation-wise. I am
>>>>>> not saying that’s what you're suggesting!
>>>>>> I agree that at the end of the day we want something that works and that is
>>>>>> understandable. To me having the semantic of dead being this can be killed if
>>>>>> the instruction does not have side effects sounded easy enough to
>>>>>> understand.
>>>>>>
>>>>>> What is your proposal for the semantic?
>>>>>>
>>>>>> (IIRC the dead flag is required for values that are never used and the
>>>>>> proposed fix somehow goes against that.)
>>>>>>
>>>>>>>
>>>>>>> - Matthias
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> llvm-dev at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>
>>>>>> _______________________________________________
>>>>>> LLVM Developers mailing list
>>>>>> llvm-dev at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
>
More information about the llvm-dev
mailing list