ZCE LLVM 14: LUI Compressed regression ??

[abukharmeh]

Hi,
I am currently evaluating the behavior of latest Zce LLVM compiler, and noticed that the compiler stopped inferring c.lui as often as it was, resulting in worsening the code size in many instances.

For example, the following assembly was generated when compiling statemate benchmark from Embench:

I wonder why ?

Thanks,
Ibrahim.

[Xinlong-Wu]

I've had a cursory look at the current code and I don't think this issue should be happening. I will try to recreate it.
Can you give me the commit id of two compilers?

[abukharmeh]

I am compiling with the head of the tree of llvm 14 zce branch with and without zce (142ad83).

For Zce screenshot on the left, I compiled with the following arguments
'-menable-experimental-extensions','-march=rv32ima_zca0p70_zcb0p70_zcmb0p70_zcmp0p70'
For mainline screenshot on the right, I compiled with the following arguments
'-march=rv32imac'

Both with -Oz optimisation level

[Xinlong-Wu]

The problem has been identified, you can try to disassemble libstatemate.o and see the difference.

This is because part of the c.lui instruction optimization (eg. global variable) is handled by the linker. When C ext is used, the argument RVC is passed via the e_flag of the elf file. But zca extension does not pass similar information.

I will add a new e_flag like RVZCA to handle that.

[abukharmeh]

Aha, that make sense, thank you very much. That actually being said, I think we should not allow linking files that have RVC flag set with the new flag as that will cause confusion !

[Xinlong-Wu]

To some extent, RVC flag is necessary for elf files. cuz the address of variables can't determine by clang compiler before linking. That means we can only decide whether convert lui to c.lui in the linker. That's why we need RVC flag.

for Zca ext, we have two ways to enable optimization lui inst.

1. let the compiler also generate RVC flag when Zce is enabled. but the llvm-objdump will read the RVC flag and set +c into target feature automatically. so I think it may cause potential errors in llvm-objdump if Zca generate RVC flag

2. Generate new RVZCA flag for Zca ext. To be honest, RVZCA and RVC have exactly the same effect. what we do in the linker is replace the code hasExtC with (hasExtC || hasExtZca). It may cause confusion, but it will not cause errors at least.

we have committed a patch as I have said in a second way for temporary. You can check it by 2f4222d

[abukharmeh]

I think I prefer to have a separate flag for Zca, because then if somebody tried to link C files with Zca files, we can error out as they are not compatible because of the change of the double float encoding meaning ?

I know that they take the allocation of flags in the elf header fairly seriously and I am not sure if they will allocate us flag for this ! Perhaps @tariqkurd-repo could comment on that ?

[tariqkurd-repo]

There certainly needs to be a compatibility check. Who allocates flags in the elf header?

[abukharmeh]

I saw this earlier https://lists.riscv.org/g/tech-toolchain-runtime/topic/80580826?p=Created%2C%2C%2C20%2C1%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2C%2C20%2C2%2C20%2C80580826 and thought you might know.
I could send an email to the mailing list, likely Jim Wilson will be able to comment on that !

[abukharmeh]

@jim-wilson I wonder if you have any comments on this issue ?

[jim-wilson]

A new elf header flag needs to be proposed to the psabi committee. We have only 32 elf header flags, and many more extensions than that, so we can't afford to have a separate elf header flag for each extension. The URL referred to above states my position, the most important extensions have elf header flags and the rest have elf attributes. See https://github.com/riscv-non-isa/riscv-elf-psabi-doc and file an issue there if you want a new elf header flag.

I haven't been following details of the zc* extensions, but it is probably wrong to use the RVC elf header flag for any zc* extension. The RVC flag is used for multiple optimizations inside the linker. Checking, I see it is used for c.j, c.jal, and c.lui. Plus the RVC flag is used to check compatibility when linking and zc* is not the same as C for that purpose.

The RVC flag is old. The elf attributes are new. Since we have the rvc info in the elf attributes also, there is a bit of redundancy here, and we could repurpose the RVC flag to mean perform c.j/c.jal/c.lui relaxations instead of indicating that the file was compiled with the C extension enabled. Those are similar but slightly difference semantics. However, this only works if there is one zc* extension that includes all of c.j, c.jal, and c.lui. And this would also require discussion in the pasbi committee.

[palmer-dabbelt]

I'm not quite sure what the issue is related to Zca, but originally the idea behind ELF flags was to just capture the ABI differences and not the ISA differences. That's why C is there, as the C stuff can have 2-byte alignment for function entry points (which isn't allowed without C, and software is free to rely on that). That said, the psABI is run quite differently these days so it's probably best to talk to folks over there.

Did any of this land in a shipped toolchain? If so we're essentially stuck with the binaries that got generated, so we'll have to go figure it out. If not then this really needs to be discussed an the psABI.

[abukharmeh]

Zc* support did not land in any shipped toolchain. Zca defines a subset of the C extension where all compressed float instructions are removed.

Its not completely clear for me weather the C ELF flag should be set when targetting Zca or not. If the C flag only indicate 2-byte alignment, then we should probably set it. However, It appears that the compiler is using that flag to indicate to the linker that it can perform various compressed linker relaxations, perhaps C attribute should be used to indicate that ?

There exist no conflict between the relaxations that the linker can perform for Zca and C extension, we just need to find a way to prevent the linker linking C extension elf files with Zca files as float could mean something else.

Maybe all we need is a new attribute, and make sure that C and Zca attributes are mutually exclusive.

I will write an email to the psABI mailing list with short description of the issue.

[kito-cheng]

Just forward my reply from the psABI mailing list:

---

Hi Ibrahim:

Short answer is we don't need new flags or attributes for all Zc* extensions, Tag_RISCV_arch is good enough for this purpose.

And let me talk more about why we don't need:

EF_RISCV_RVC is used for checking the compatibility, that is also used
for linker checking OK to do relaxation to transform instruction into
compressed form or not, and, zc* will require 2 bits if using the same
scheme (ELF flags scheme) - one for zca and one for zcmt.

However this mechanism is not scalable, ELF flags only have 32 bits
and we used 5 bits now, so we allocate ELF flags only if that is
really necessary and no other alternatives; we are migrating to ELF
attributes for record architecture extension information in the past
few years, that should let linkers have enough information to do the
right linker relaxation, but binutils are still using EF_RISCV_RVC
rather than ELF attribute to determine what kind of relaxation can be
used, so I guess that requires a few more engineering efforts to
implement at that.

Here is another problem behind the solution, the ELF attribute isn't
mandatory for current psABI spec, so that would be TODO item for psABI
spec, but I think that's wound be big problem in current open source
toolchain since we already default to emit those attribute for both
LLVM and GCC compiler.

• Definition of EF_RISCV_RVC from psABI spec:
  https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc

This bit is set when the binary targets the C ABI, which allows
instructions to be aligned to 16-bit boundaries (the base RV32 and
RV64 ISAs only allow 32-bit instruction alignment). When linking
objects which specify EF_RISCV_RVC, the linker is permitted to use RVC
instructions such as C.JAL in the relaxation process.

[abukharmeh]

@kito-cheng Thank you very much. So I understand that we should really set EF_RISCV_RVC flag for Zc* extension as ideally its used to indicate 2-byte alignment, and we should use Tag_RISCV_arch to check for compatibility and what linker relaxations we are performing !

[abukharmeh]

Here is another problem behind the solution, the ELF attribute isn't
mandatory for current psABI spec, so that would be TODO item for psABI
spec, but I think that's wound be big problem in current open source
toolchain since we already default to emit those attribute for both
LLVM and GCC compiler.

So our working assumption should be that psABI TG will make them mandatory at some point soon, and we dont need to worry about that ?

[Xinlong-Wu]

Jiawei @pz9115 will replace Sinan for the GCC updates of Zc

[pz9115]

We had use EF_RISCV_RVC in binutils, when reuse instruction in c extension, this will enable the compress intrustion generate correctly. The whole Zc* extension can be devided in two part, one (Zca Zcf) reuse c extension instructions, another part (Zcb Zcm*) are independent. For the reuse part, it need to submit the feature in original c elf flag I think.

[kito-cheng]

So our working assumption should be that psABI TG will make them mandatory at some point soon, and we dont need to worry about that ?

Yes :)