[PATCH] D152945: [SystemZ][z/OS] Implement executePostLayoutBinding for GOFFWriter
Neumann Hon via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 28 14:14:35 PDT 2023
Everybody0523 added inline comments.
================
Comment at: llvm/lib/MC/GOFFObjectWriter.cpp:705
+ Flags Attr[10];
+ void setAmode(GOFF::ESDAmode Amode) { Attr[0].set(0, 8, Amode); }
+ void setRmode(GOFF::ESDRmode Rmode) { Attr[1].set(0, 8, Rmode); }
----------------
DiggerLin wrote:
> Everybody0523 wrote:
> > DiggerLin wrote:
> > > I am expressing the difficulty in understanding the meaning or significance of the values 0, 8 in a particular context. it's possible to use a "static constexpr" to define these values to provide more clarity and context?
> > >
> > > if can , please change all the following .
> > >
> > > for example: in xcoff.h
> > >
> > > constexpr size_t FileNamePadSize = 6;
> > > constexpr size_t NameSize = 8;
> > > constexpr size_t FileHeaderSize32 = 20;
> > > constexpr size_t FileHeaderSize64 = 24;
> > >
> > So in the highlighted function (`setAmode`), we're setting the address mode of the symbol, which is an 8-bit field. The first parameter of the `set()` function is the offset to which we begin, the second parameter is the number of bits that should be written, and the last value is the actual value to write, so in this case we just mean "0-bit offset" and "write 8 bits" for the 0 and 8 respectively.
>
> sorry that I do not express my comment clearly. My worry about is that your code is not easy to understand without reading the GOFF document.
>
> can we change the code something like ?
>
> ```
> struct SymbolBehavAttr {
> uint8_t AddressingProperties;
> uint8_t ResidenceProperties;
> uint8_t TextRecordtyle:4;
> uint8_t BindingAlgorithm:4;
> ....
> } ;
> ```
> (and if the SymbolBehavAttr will be by other functionality later, we can put it GOFF.h ?)
>
> and then you do not need the API setRmode() and setAmode()
> and you can assign the value directly
>
> SymbolBehavAttr BehavAttr;
> BehavAttr.AddressingProperties = ...
>
Hi, so I guess you would want something like this?
```
LLVM_PACKED_START
struct SymbolBehavAttr {
uint8_t AddressingProperties;
uint8_t ResidenceProperties;
uint8_t TextRecordStyle:4; //Byte 2 begin
uint8_t BindingAlgorithm:4;
uint8_t TaskingBehavior:3; //Byte 3 begin
uint8_t ReservedSpace0:1;
uint8_t ReadOnlyIndicator:1;
uint8_t Executable:3;
uint8_t ReservedSpace1:2; //Byte 4 begin
uint8_t DuplicateSymbolSeverity:2;
uint8_t BindingStrength:4;
uint8_t ClassLoadingBehavior:2; //Byte 5 begin
uint8_t CommonFlag:1;
uint8_t IsDirectReference:1;
uint8_t BindingScope:4;
uint8_t ReservedSpace2:3; //Byte 6 begin
uint8_t LinkageType:1;
uint8_t Alignment:5;
uint8_t ReservedSpace3;
uint16_t ReservedSpace4;
};
LLVM_PACKED_END
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152945/new/
https://reviews.llvm.org/D152945
More information about the llvm-commits
mailing list