search

riscv-non-isa / riscv-c-api-doc

Documentation of the RISC-V C API
https://jira.riscv.org/browse/RVG-4
Creative Commons Attribution 4.0 International
68 stars 38 forks source link

Add -m[no-]unaligned-access #62

Closed wangpc-pp closed 7 months ago

wangpc-pp commented 8 months ago

These two options are negative equivalent to -m[no-]strict-align. Clang/LLVM has supported both ways (same as AArch32/AArch64/LoongArch). This can reduce some miscompiles when migrating some existed software packages.

wangpc-pp commented 8 months ago

I think this patch is missing an explanation of why we'd want two ways of doing the same thing. It's probably clear to you, or else you wouldn't have proposed it, but it isn't clear to me.

Oh sorry! The background is: Clang/LLVM has supported both ways (same as AArch32/AArch64/LoongArch).

def munaligned_access : Flag<["-"], "munaligned-access">, Group<m_Group>,
  HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64/LoongArch/RISC-V only)">;
def mno_unaligned_access : Flag<["-"], "mno-unaligned-access">, Group<m_Group>,
  HelpText<"Force all memory accesses to be aligned (AArch32/AArch64/LoongArch/RISC-V only)">;

def mstrict_align : Flag<["-"], "mstrict-align">, Alias<mno_unaligned_access>,
  Flags<[HelpHidden]>, Visibility<[ClangOption, CC1Option]>,
  HelpText<"Force all memory accesses to be aligned (same as mno-unaligned-access)">;
def mno_strict_align : Flag<["-"], "mno-strict-align">, Alias<munaligned_access>,
  Flags<[HelpHidden]>, Visibility<[ClangOption, CC1Option]>,
  HelpText<"Allow memory accesses to be unaligned (same as munaligned-access)">;

This can reduce some miscompiles when migrating some existed software packages. And I just posted the GCC implementation recently.

aswaterman commented 8 months ago

@wangpc-pp I am not qualified to comment on whether that's a sufficient justification, but my point was, the explanation needs to go into the document. Perhaps it could be a non-normative note, but regardless, readers will want to understand why there are two ways to do the same thing.

wangpc-pp commented 8 months ago

Ping for comments. :-)

MaskRay commented 8 months ago

I think this patch is missing an explanation of why we'd want two ways of doing the same thing. It's probably clear to you, or else you wouldn't have proposed it, but it isn't clear to me.

Oh sorry! The background is: Clang/LLVM has supported both ways (same as AArch32/AArch64/LoongArch).

I don't think this is sufficient motivation.

It's unfortunate that Clang uses Alias and not make the options target-specific. Well, if an architecture wants just one spelling, we can make the option target-specific and drop Alias.