Cfi update - #138
Conversation
Converted the clarification updates from 003-clarification-cfi.txt into markdown. Then merged them with the parts from 013-generalize-address-spaces.md that were left out of the address space proposal. Then merged inHeavy editing of CFI updates the new operator from 017-call-frame-entry-registers.txt into one pro posal that handles alConverted the clarification updates from 003-clarification-cfi.txt l the CFI loosinto markdown. Then merged them with the parts from e ends. Began addi013-generalize-address-spaces.md that were left out of the address ng something like the operation headers tspace proposal. Then merged in the new operator from o the CFA Instru017-call-frame-entry-registers.txt into one proposal that handles all ctions. the CFI loose ends. Began adding something like the operation headers to the CFA Instructions. Signed-off-by: Ben Woodard <woodard@redhat.com>
Signed-off-by: Ben Woodard <woodard@redhat.com>
More work on the CFI proposal. Still WIP. Signed-off-by: Ben Woodard <woodard@redhat.com>
New intro -- much better. Kind of brings the whole topic together. Begin adding prototype like headers to rules. Signed-off-by: Ben Woodard <woodard@redhat.com>
markus-metzger
left a comment
markus-metzger
left a comment
There was a problem hiding this comment.
I stopped reviewing at some point. I think this needs the high-level design described in the introduction.
Going through the changes, I suspect that the intent is to turn CFA into a location and make all CFI rules yield locations. I'm not convinced that this is strictly necessary. It seems to add complications and turns simple and clear rules into something rather complicated.
Defining CFA as an integral offset in some ABI-defined address-space (which could be some generic address-space covering multiple address-spaces) would leave most of CFI intact. I would like to see an example where this simpler approach would not work.
| > *Previously, the Call Frame Location, CFL, was known as the Call | ||
| > Frame Address, CFA. However, GPUs do not necessarily store | ||
| > previous frames on the stack, they may pushed to some local | ||
| > storage in the GPU. So it is not quite to appropriate to assume |
There was a problem hiding this comment.
Wouldn't that 'local storage in the GPU' also behave like a stack? Are we just talking about different address-spaces or something more exotic?
|
|
||
| With: | ||
|
|
||
| > An area of memory that is allocated on a stack called a “call |
There was a problem hiding this comment.
I'm not convinced that CFA needs to be a location and cannot be an integral offset in an ABI-defined address-space that contains the stack.
| with "Call Frame Location" in the paragraph that describes what the | ||
| CFL is. | ||
|
|
||
| > The previous value of this register is the undefined location |
There was a problem hiding this comment.
It is not clear where this is to be added. Looks like it is replacing the 'undefined' rule. I think it is fine as it is unless we want to change all CFI register rules to produce a location. The introduction did not say anything like that (or I didn't find or understand it).
| requirements that are different than CPUs. In particular, they have | ||
| multiple address spaces and and can spill registers to them. Some | ||
| architectures of GPUs can even spill registers to other registers | ||
| which are farther away in their memory hierarchy. The fact that GPUs |
There was a problem hiding this comment.
What does 'farther away in their memory hierarchy' mean in the context of registers?
| multiple address spaces and and can spill registers to them. Some | ||
| architectures of GPUs can even spill registers to other registers | ||
| which are farther away in their memory hierarchy. The fact that GPUs | ||
| use various locations to store previous frames which cannot be |
There was a problem hiding this comment.
What does 'use various locations to store previous frames' mean? Can we give a concrete real-world example?
| following: | ||
|
|
||
| > The previous value of this register is located either in a generic | ||
| > value or in implicit storage produced by evaluating the DWARF |
There was a problem hiding this comment.
If the producer wanted to use implicit storage, why did it not use the 'expression(E)' rule?
I see no issue with restricting 'val_expression(E)' to values that can be pushed onto the stack and leave the rule to define the value of the register as the top stack element after evaluating E.
Another big chunk of edits as I pound this into shape. Several of the edits needed discussion by the group and needed to be resolved there. Thanks to Markus Metzger for his review comments. Several of them have been taken care of but several still need to be worked through and resolved. Signed-off-by: Ben Woodard <woodard@redhat.com>
| which are farther away in their memory hierarchy. The fact that GPUs | ||
| use various locations to store previous frames which cannot be | ||
| expressed as a simple address requires the generalization of Call | ||
| Frame Addresses or Cononical Frame Addresses CFA (it is used |
| section and the .debug_frames section for the target GPU in its | ||
| embedded ELF file. If there were an architecture where the host's ELF | ||
| file's .debug_frames section mixed addresses between host code and | ||
| device code, then additional Row Creation Instructions would be need |
There was a problem hiding this comment.
Typo: "would be need" -> "would need"
| In bullet point 3 Current Compilation Unit add the following paragraph | ||
| between the normative text and the non-normative text. | ||
|
|
||
| > DWARF exppressions used in CFI (see Section 7.4.2 on page 198) do |
| > DWARF exppressions used for variable's location or value do not | ||
| > have a curret register in their context. |
There was a problem hiding this comment.
Typo: exppressions, curret
| To: | ||
|
|
||
| > *A DWARF expression may be able to be evaluated without a thread, | ||
| > call frame, lane, program counter, current object, registeer, or |
| > the current state of the register. However, if the current frame | ||
| > is not the top most frame, then operators such as DW_OP_reg<N>, | ||
| > DW_OP_regx, DW_OP_breg<N> and DW_OP_bregx will access the | ||
| > previous contents of the register by applying the CFI rules which |
| In the description of DW_OP_breg0...DW_OP_breg32 after "register | ||
| (0-31)" add the following qualifier: "at the time of the Current Frame | ||
| (See 3.1 page 47) are virtually restored by CFI (see 7.4 page 193)" | ||
|
|
||
| In section 3.8 replace: | ||
|
|
||
| > The following DWARF operations can be used to specify a register | ||
| > location. | ||
|
|
||
| With: | ||
|
|
||
| > The following DWARF operations can be used to specify a register | ||
| > location. When the Current Frame (See 3.1 page 47) in the | ||
| > Expression Evaluation Context is not the top most frame, then the | ||
| > state of these registers is first virtually restored using CFI | ||
| > (See section 7.4 page 193) and that value is referenced when | ||
| > evaluating these operations. |
There was a problem hiding this comment.
Below, there is a new paragraph that starts with "When a consumer is evaluating a DWARF expression in the context...". If that paragraph is added, do we still need these changes?
| For the "val_offset(N)" register rule, replace the description with | ||
| the following: | ||
|
|
||
| > N is a signed byte offset. The previous value of this register is | ||
| > saved at the memory address pointed to by the location description | ||
| > L, where L is the location description of the current CFL offset by | ||
| > N bytes. |
There was a problem hiding this comment.
This is the "offset" rule, not "val_offset'.
| For the "val_offset(N)" register rule, replace the description with | ||
| the following: | ||
|
|
||
| > N is a signed byte offset. The previous value of this register is |
There was a problem hiding this comment.
How about this rewording?
The previous value of this register is saved at CFL, offset by N bytes.
| > *For example, if a register R1 has a `DW_CFA_def_cfa_expression` | ||
| > instruction that evaluates a `DW_OP_call_frame_entry_reg` | ||
| > operation that specifies register R2, and register R2 has a | ||
| > `DW_CFA_def_cfa_expression` instruction that that evaluates a |
3 participants
Heavy editing of CFI updates
Merged all the proposals which deal with CFI back into one bigger proposal.