This is an archive of the discontinued LLVM Phabricator instance.

[SystemZ][z/OS] Implement yaml2obj for GOFF Object File Format
Needs ReviewPublic

Authored by Everybody0523 on May 15 2023, 2:06 PM.

Details

Summary

This patch implements yaml2obj for the GOFF Object File Format.

Diff Detail

Event Timeline

Everybody0523 created this revision.May 15 2023, 2:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2023, 2:06 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
Everybody0523 requested review of this revision.May 15 2023, 2:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2023, 2:06 PM

Fix issue with tests, hopefully.

kpn added a comment.May 17 2023, 11:31 AM

There's a lot of code here. I'll try to look at it in the next couple of weeks. I was out last week and I'm a little behind.

llvm/include/llvm/BinaryFormat/GOFF.h
33

Has the spec been updated in the past three years? My z/OS 2.4 copy doesn't say that RLD records have an unsigned length field.

Also, if the spec is wrong about TXT cards then correcting the spec seems helpful. I don't know how difficult this is in practice.

Everybody0523 added inline comments.May 23 2023, 8:56 AM
llvm/include/llvm/BinaryFormat/GOFF.h
33

The spec isn't wrong per-say, this is essentially a work-around for a binder bug.

kpn added inline comments.Aug 1 2023, 12:02 PM
llvm/include/llvm/BinaryFormat/GOFF.h
33

A rewording of the comment so it says there's a binder bug instead of saying, essentially, that the spec is wrong would be helpful.

llvm/include/llvm/ObjectYAML/GOFFYAML.h
35

This appears to be unused. I don't know what "ERST" stands for, either.

36

Perhaps a comment on what "RQW" means? It looks like it masks out bits that are currently reserved.

53

Is this undocumented?

55

Typo: Addressing

56

How does this work? The other flags are bits in the same byte.

llvm/lib/ObjectYAML/GOFFEmitter.cpp
250

I couldn't find any documentation on any of this aside from the Architecture Level field. Same issue with the module properties field. So I can't say anything about the correctness of this code.

309

Shouldn't this be the duplicate symbol severity? What is ESD_BA_NoPrime?

llvm/lib/ObjectYAML/GOFFYAML.cpp
222

Addressing. This is probably in other places as well.