Re: [Patch,AVR,trunk,4.7] PR52461: Fix RAMPZ clobbering and RAMP* in epilogue

[Georg-Johann Lay <avr at gjlay dot de>]

Richard Guenther wrote:
> On Mon, Mar 5, 2012 at 6:54 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
>> Richard Guenther wrote:
>>> On Mon, Mar 5, 2012 at 6:15 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
>>>> Richard Guenther wrote:
>>>>> On Mon, Mar 5, 2012 at 4:56 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
>>>>>> Richard Guenther wrote:
>>>>>>> On Mon, Mar 5, 2012 at 4:25 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
>>>>>>>> Richard Guenther wrote:
>>>>>>>>
>>>>>>>>> All commits to the 4.7 branch need explicit release manager approval.  AVR
>>>>>>>>> isn't primary/secondary so please do not change anything before is
>>>>>>>>> released 4.7.0 for it.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Richard.
>>>>>>>> What is the exact procedure in that case?
>>>>>>>> Wait until approve from release manager in that case?
>>>>>>>> Who is the release manager, and should I CC for such changes?
>>>>>>>> Or just hope the patch is not overseen.
>>>>>>> The exact procedure is to do bugfixing during stage3/4, for release blockers
>>>>>>> that pop up after a release candidate is created (like now), CC a release
>>>>>>> manager (Jakub, me, Joseph) for patches that you like to get in even
>>>>>>> though the branch is frozen.  Usually only bugs that prevent basic functionality
>>>>>>> (like building a target) can be fixed at this point, for everything
>>>>>>> else you have
>>>>>>> to wait until after 4.7.0 is released and the branch opens again for regression
>>>>>>> fixes.
>>>>>>>
>>>>>>> Richard.
>>>>>> I was not aware that the 4.7.0 branch is completely frozen for the next 3
>>>>>> weeks; I thought the usual rules for backporting patches do apply...
>>>>> No they don't.  How would you expect that testing a release candidate would
>>>>> work if we put in any not strictly necessary changes?  That would make a
>>>>> release candidate quite pointless.
>>>>>
>>>>>> The patch changes only in libgcc/config/avr and gcc/config/avr
>>>>>>
>>>>>> The patch does not fix a blocker in the sense that without it avr cannot be
>>>>>> built, but the changes are essential.
>>>>> Surely not so essential as that they cannot be put in place to make the 4.7.1
>>>>> release then.
>>>> Okay.
>>>>
>>>> In that case I'd like to add a note to the caveats section in wwwdocs
>>>>
>>>> ./gcc-4.7/changes.html
>>>>
>>>> that the avr-gcc 4.7.0 is not intended for public consumption and because of
>>>> developer shortage at least 4.7.1 should be used.
>>> Completely unusable?  It looks like it only affects a subset of all devices:
>>> "To read from flash on devices with more than 64KiB of flash"
>> The patch fixes more problems than indicated by log message's PR.
>>
>>> It sounds like a random wrong-code bug, which do happen.
>> Yes they happen. But if the defect is so severe that the code is effectively
>> useless, the compiler is useless.
>>
>> That is not a problem if it is clearly stated and people don't waste days to
>> set up and distribute avr toolchains that are useless. It would simply be not
>> fair to let them waste their time and to hold back that knowledge.
>>
>>> There is just a timeframe where random fixes are not good.
>> This is not a problem. Just inform the potential users about the defect.
>>
>>> Was 4.6.3 "intended for public consumption"?
>> Yes.
> 
> So, what patch regressed state from 4.6.3 to the present completely unusable
> compiler?  Why is it not appropriate to revert that instead?  Why is the
> bug not marked as regression?  The bug sounds as if it only applies to
> XMEGA+EBI (whatever that means).

Yes, the problems all affect new features.

> Can you split the patch and separate out the fix for the PR if the patch
> does not only fix that PR?

You are right, it's definitely better to break up the patch into the individual
PRs if there is no way to get a patch in 4.7.0, anyway.

As far as I can see, only new features/architectures are non-functional to that
postponing them to 4.7.1 is an option.

Johann