Messages in this thread | | | Date | Thu, 20 Nov 2008 16:57:44 +0100 | From | Alexander van Heukelum <> | Subject | Re: [PATCH] x86: clean up after: move entry_64.S register saving out of the macros |
| |
On Thu, Nov 20, 2008 at 04:39:54PM +0100, Ingo Molnar wrote: > > * Alexander van Heukelum <heukelum@mailshack.com> wrote: > > > On Thu, Nov 20, 2008 at 04:04:12PM +0100, Ingo Molnar wrote: > > > > > > * Alexander van Heukelum <heukelum@mailshack.com> wrote: > > > > > > > This add-on patch to x86: move entry_64.S register saving out of the > > > > macros visually cleans up the appearance of the code by introducing > > > > some basic helper macro's. It also adds some cfi annotations which > > > > were missing. > > > > > > > > Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm> > > > > --- > > > > arch/x86/kernel/entry_64.S | 220 ++++++++++++++++++++++---------------------- > > > > 1 files changed, 112 insertions(+), 108 deletions(-) > > > > > > > > Hello Ingo, > > > > > > > > This patch improves the CFI-situation in entry_64.S, but restricted > > > > mostly to the areas touched by "x86: move entry_64.S register saving > > > > out of the macros". I'm sure there will be some small errors > > > > somewhere, but it compiles and runs fine. > > > > > > very nice cleanup! This is exactly what should be done. Applied to > > > tip/x86/irq. > > > > > > Note, i did a small rename: > > > > > > cfi_pushq => pushq_cfi > > > cfi_popq => popq_cfi > > > cfi_store => movq_cfi
Does not work... But if you are attached to the underscores, I think we can force it to work by using CPP to convert it to something the assembler does parse right:
#define pushq_cfi pushq.cfi
etc?
Or is that just too ugly?
Alexander
> > > as the goal is to have the actual source code read mostly as regular > > > assembly code. The fact that the macro is equivalent to a > > > default-annotated pushq/popq/movq instruction is much more important > > > than the fact that it also does CFI annotations. > > > > > > Also, while cfi_store is correct as well, the usual x86 assembly term > > > (and instruction used here) is movq. > > > > Now I have a little problem with my next patch... I wanted to > > introduce cfi_load. Guess what assembly instruction that maps to ;). > > heh ;-) > > the restore direction could be named movq_cfi_restore, and have the same > order of arguments as the regular movq that it replaces. I.e.: > > movq 8(%rsp),%r11 > CFI_RESTORE r11 > > would map to: > > movq_cfi_restore 8, r11 > > or so. > > cfi_store has really a bad name: it's confusing whether it's the CFI > info we are storing/registering (which we are), or a 'store' instruction > (which this is too). > > If then we should name it movq_cfi_store or movq_cfi_register - but > that's too long. > > movq_cfi for the frame construction direction and movq_cfi_restore for > the frame deconstruction phase sounds like a good naming compromise, hm? > > Ingo
-- Alexander van Heukelum
| |