[PATCH] D69097: [AArch64][MachineOutliner] Return address signing for outlined functions

Tue Oct 29 09:09:31 PDT 2019

ostannard added inline comments.

================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-v8-3.ll:61
+; CHECK-NEXT:               .cfi_negate_ra_state
+; CHECK:                    bl OUTLINED_FUNC
+  %1 = alloca i32, align 4
----------------
tellenbach wrote:
> ostannard wrote:
> > I don't think this is correct: `@c` is being compiled for Armv8.0-A, but we've introduced a call to a function with the `retab` instruction, so it will fail if run on an v8.0-A machine. I think we need to check that all of the outlining candidate functions are being compiled for v8.3-A, or carefully construct the new function's subtarget to be a subset of the candidate functions.
> I see your point. Maybe this test is just too artificial and in reality functions in one module will all be compiled for the same subtarget? I think the machine outliner is not the correct place to check if this is true. 
> 
> And if we would like to check it here this would be a more general issue and should probably addressed in a separate patch.
Subtargets can vary per-function, this might happen because the user is doing runtime checks, and dispatching to optimised versions using later architecture features when possible. Fixing this as a separate patch probably makes sense, but it will nee to be committed before this one otherwise we'll start generating invalid code.

I guess this hasn't been a problem for the outliner until now, because doesn't introduce any instructions not present in v8.0-A.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69097/new/

https://reviews.llvm.org/D69097