Implement "-march=rv32ib_Zbb_Zbc_...ZbX" ISA subset selection

[mablinov]

Hi all,

I have put together a small patchset which implements command-line bitmanip ISA subset selection. I've tried to follow the current bitmanip draft spec as closely as possible. In particular, the 'B' bitmanip subset is taken as the one indicated by the extended dotted line (everything excluding Zbt, Zbf.)

Please also read the "Problems" section at the bottom.

The bitmanip spec currently defines 9 subgroups of instructions. I defined them as target flags residing in a new target variable called "x_riscv_bitmanip_flags", each called accordingly:

OPTION_MASK_BITMANIP_ZBB
OPTION_MASK_BITMANIP_ZBC
OPTION_MASK_BITMANIP_ZBE
OPTION_MASK_BITMANIP_ZBF
OPTION_MASK_BITMANIP_ZBM
OPTION_MASK_BITMANIP_ZBP
OPTION_MASK_BITMANIP_ZBR
OPTION_MASK_BITMANIP_ZBS
OPTION_MASK_BITMANIP_ZBT

When invoking gcc as follows: $ riscv32-unknown-elf-gcc -O2 -march=rv32ib -mabi=ilp32 -S -o gcc-demo.s gcc-demo.c

the flag states become

ZBB, ZBC, ZBE, ZBF, ZBM, ZBP, ZBR, ZBS, ZBT, MASK_BITMANIP
1    1    1    0    1    1    1    1    0    1

If the user provides atleast one sub-ISA specifier, then only the sub-ISA flags are honoured. e.g.: $ riscv32-unknown-elf-gcc -O2 -march=rv32ib_Zbb -mabi=ilp32 -S -o gcc-demo.s gcc-demo.c

will set

ZBB, ZBC, ZBE, ZBF, ZBM, ZBP, ZBR, ZBS, ZBT, MASK_BITMANIP
1    0    0    0    0    0    0    0    0    1

and all following sub-ISA specifiers are simply additive, e.g.: $ riscv32-unknown-elf-gcc -O2 -march=rv32ib_Zbb_Zbf_Zbt -mabi=ilp32 -S -o gcc-demo.s gcc-demo.c

will set

ZBB, ZBC, ZBE, ZBF, ZBM, ZBP, ZBR, ZBS, ZBT, MASK_BITMANIP
1    0    0    1    0    0    0    0    1    1

and so forth. The user must provide the 'b' keyword before adding (if any) "ZbX" directives, and the "ZbX" directives must always follow directly after the 'b' directive. The "ZbX" directives must always have atleast one set of underscores surrounding them. If there are multiple "ZbX" directives, they must come one after the other.

The "MASK_BITMANIP" target macro is still there, but it is not used in the bitmanip.md conditions.

There are 3 patches. Each can be applied without breaking the build, but they must come in the right order. Patch 1: add riscv.opt "riscv_bitmanip_flags" variable, and associated masks. Patch 2: modify bitmanip.md insns to only get generated if the associated subset mask is set. Patch 3: modify riscv-common.c to accept "ZbX" form of subset ISA flags.

I've also provided a patch that applies all of them at once.

There is almost certainly some fiddling to be done with the riscv.c file aswell, and I suspect there to be a few corner cases in the parser, but this is useful as it is and would like to see what people think of the current implementation.

Output assembly arch attributes look like this: .attribute arch, "rv32i2p0_b2p0_Zbb2p0_Zbc2p0_Zbt2p0_Zbp2p0"

Patches: 0001-add-bmi-subisa-march-opts.patch.txt 0002-add-bmi-subisa-march-bitmanip.patch.txt 0003-add-bmi-subisa-march-common.patch.txt add-bmi-subisa-march-all.patch.txt

Problems:

1. Canonical flags order.

No order of flags is currently enforced. What is the canonical order of the ZbX flags? Alphabetical? Or something else?

2. Redundant flag names.

Unfortunately, GCC refuses to create target masks relative to a specified variable if a flag name is not provided. I am referring to the riscv.opt file.

Take for example the ZBB directive:

...
mbmi-zbb
Target Mask(BITMANIP_ZBB) Var(riscv_bitmanip_flags)
Support the base subset of the Bitmanip extension.
...

This causes the following code to be generated in build/gcc/options.h:

#define OPTION_MASK_BITMANIP_ZBB (HOST_WIDE_INT_1U << 0) // <<<<<<<<<<<<<<<<
#define OPTION_MASK_BITMANIP_ZBC (HOST_WIDE_INT_1U << 1)
#define OPTION_MASK_BITMANIP_ZBE (HOST_WIDE_INT_1U << 2)
#define OPTION_MASK_BITMANIP_ZBF (HOST_WIDE_INT_1U << 3)
#define OPTION_MASK_BITMANIP_ZBM (HOST_WIDE_INT_1U << 4)
#define OPTION_MASK_BITMANIP_ZBP (HOST_WIDE_INT_1U << 5)
#define OPTION_MASK_BITMANIP_ZBR (HOST_WIDE_INT_1U << 6)
#define OPTION_MASK_BITMANIP_ZBS (HOST_WIDE_INT_1U << 7)
#define OPTION_MASK_BITMANIP_ZBT (HOST_WIDE_INT_1U << 8)
#define MASK_DIV (1U << 0)
#define MASK_EXPLICIT_RELOCS (1U << 1)
#define MASK_FDIV (1U << 2)
#define MASK_SAVE_RESTORE (1U << 3)
#define MASK_STRICT_ALIGN (1U << 4)
#define MASK_64BIT (1U << 5)
#define MASK_ATOMIC (1U << 6)
#define MASK_BITMANIP (1U << 7)
#define MASK_DOUBLE_FLOAT (1U << 8)
#define MASK_HARD_FLOAT (1U << 9)
#define MASK_MUL (1U << 10)
#define MASK_RVC (1U << 11)
#define MASK_RVE (1U << 12)

We probably don't want to expose this dual method of specifying subisas, so let's try removing the -mbmi-zbb name from riscv.opt:

...
Target Mask(BITMANIP_ZBB) Var(riscv_bitmanip_flags)
...

This causes the following output in build/gcc/options.h:

#define OPTION_MASK_BITMANIP_ZBC (HOST_WIDE_INT_1U << 0)
#define OPTION_MASK_BITMANIP_ZBE (HOST_WIDE_INT_1U << 1)
#define OPTION_MASK_BITMANIP_ZBF (HOST_WIDE_INT_1U << 2)
#define OPTION_MASK_BITMANIP_ZBM (HOST_WIDE_INT_1U << 3)
#define OPTION_MASK_BITMANIP_ZBP (HOST_WIDE_INT_1U << 4)
#define OPTION_MASK_BITMANIP_ZBR (HOST_WIDE_INT_1U << 5)
#define OPTION_MASK_BITMANIP_ZBS (HOST_WIDE_INT_1U << 6)
#define OPTION_MASK_BITMANIP_ZBT (HOST_WIDE_INT_1U << 7)
#define MASK_DIV (1U << 0)
#define MASK_EXPLICIT_RELOCS (1U << 1)
#define MASK_FDIV (1U << 2)
#define MASK_SAVE_RESTORE (1U << 3)
#define MASK_STRICT_ALIGN (1U << 4)
#define MASK_64BIT (1U << 5)
#define MASK_ATOMIC (1U << 6)
#define MASK_BITMANIP (1U << 7)
#define MASK_DOUBLE_FLOAT (1U << 8)
#define MASK_HARD_FLOAT (1U << 9)
#define MASK_MUL (1U << 10)
#define MASK_RVC (1U << 11)
#define MASK_RVE (1U << 12)
#define MASK_BITMANIP_ZBB (1U << 13) // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

It's now relative to the general-purpose target_flags variable, rather than the riscv_bitmanip_flags. Is there is some magic combination of GCC option properties to get around this?

[asb]

If the user provides atleast one sub-ISA specifier, then only the sub-ISA flags are honoured

This seems a weird semantic. Why not just -march=rv32i_Zbb etc? This keeps extensions additive.

[mablinov]

This seems a weird semantic. Why not just -march=rv32i_Zbb etc? This keeps extensions additive.

I realize now that I had a misunderstanding about what the 'b' flag in the "-march" string represented - I treated it as an indicator that "this architecture makes use of the bitmanip spec", and thought that the capital 'B' subset specifier in the bitmanip document was a separate thing.

Will fix all this behaviour in next patch.

[cliffordwolf]

Merged it into my gcc.diff, and made some quick changes in 7d57261 to make the flags additive.

Talking about gcc.diff: Clearly that's a horrible solution going forward! I think it would make sense to have git repositories for at least gcc and binutils, with riscv-bitmanip branches in them. If for no other reason than simply so that we have a proper git history so we know whose work is even in there. cc @jim-wilson @aswaterman

[aswaterman]

Hey @cliffordwolf, hope you’re well. I would suggest using branches instead of diffs. Do you have the write access you require?

[cliffordwolf]

Hmm.. I could swear https://github.com/riscv/riscv-gcc didn't exist when I created gcc.diff. Is it a relatively new repo? I don't have write access to that repo though. I think riscv-isa-sim is the only other repo I have write access to.

I think we should use a consistent name for the branch in all repos. Any suggestions? Just bitmanip? Or maybe riscv-bitmanip?

[aswaterman]

It has been there for a long time. But note that it’s merely a holding cell; when things should go upstream, they should be submitted through the GNU mailing lists, not via PRs to this repo.

I will always agree with any attempt to establish naming consistency, but will leave that matter to you.

Send me an email if you find that you don’t have write access.

[mablinov]

@CliffordWolf Hi Clifford, you probably really don't want this version - it does parsing the non-standard way with -mbmi-zbX flags. I only really threw this in at the time just so people had some handles for running size benchmarks for different ZbX subsets.

The version attached below actually does it through the -march directive, e.g. -march=rv32imafdc_zbb_zbc, and is also additive.

zparse-for-bitmanip.diff.txt

It still lags some functionality that has been very recently discussed (-misa-spec, and reordering parsing order), but it should be ok for bitmanip work for now.

[cliffordwolf]

I reverted 7d57261 and 7266bf2. We'll move the gcc patch to a gcc github branch soon. Let's merge your new patch when this is done. Leaving this issue open until then.