[llvm] r298734 - [Outliner] Remove no red zone requirment for AArch64
Jessica Paquette via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 29 14:24:41 PDT 2017
Sorry about that. I was both deeply wrong *and* deeply tired when I committed that.
I’ve included the (incorrect) change to the test case in machine-outliner.ll for the sake of consistency with the reverted commit. The only change is that noredzone is no longer an attribute. Both of the functions in the test are leaf functions, so they both use the red zone after this change. This would have passed, because the outlined functions would still have been outlined as tail calls. Thus, there are no modifications to the stack at those calls.
If a function was outlined from a function using a red zone, and that outlined function *was not* a tail call, then the red zone would have been corrupted. That is what I *should have* immediately written a test case for. Mistake on my part. The correct test, for future similar patches, should ensure that sequences outlined from functions with red zones don’t overwrite any data stored in the red zone.
(I didn’t remove the noredzone requirement from the X86 outliner in that commit, so there was no change in functionality there.)
In the future, I’ll be less trusting of my judgement with these things. :)
- Jessica
==============================================================================
--- a/test/CodeGen/AArch64/machine-outliner.ll
+++ b/test/CodeGen/AArch64/machine-outliner.ll
@@ -40,4 +40,4 @@ define void @dog() #0 {
; CHECK-NEXT: ret
-attributes #0 = { noredzone nounwind ssp uwtable "no-frame-pointer-elim"="false" "target-cpu"="cyclone" }
+attributes #0 = { nounwind ssp uwtable "no-frame-pointer-elim"="false" "target-cpu"="cyclone" }
> On Mar 29, 2017, at 1:48 PM, Davide Italiano <davide at freebsd.org> wrote:
>
> On Fri, Mar 24, 2017 at 4:28 PM, Davide Italiano <davide at freebsd.org> wrote:
>> On Fri, Mar 24, 2017 at 1:47 PM, Jessica Paquette via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>> Author: paquette
>>> Date: Fri Mar 24 15:47:59 2017
>>> New Revision: 298734
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=298734&view=rev
>>> Log:
>>> [Outliner] Remove no red zone requirment for AArch64
>>>
>>> AArch64 doesn't require -mno-red-zone; stack fixups are sufficient here. This was
>>> unnecessarily copied over from the X86 target.
>>>
>>> (You can now outline with red zones! Yay!)
>>>
>>> Removing the requirement passes all Single/MultiSource tests.
>>>
>>>
>>
>> Commenting on this one, even though you reverted. This is clearly a
>> functional change and show be accompanied by a test, same for the X86
>> bits. Can you please add one?
>>
>
> ping?
>
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
More information about the llvm-commits
mailing list