LGTM, and m in GCC has some special treatment for vector types, it will forbid offset, but it's kind of because GCC's implementation, so I am not asking to match this behavior.

@jobnoorman could you rebase again? I got conflict during apply this patch

@jobnoorman do you need some help to commit this and D144384? let me know if you need help, and I would suggest you could drop a mail to Chris to obtain write access https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access :)

• Update relocation number to 60 and 61.
• Move testcase to fixups-expr-uleb128.s
• Reserved enough bytes when the symbol can't evaluated at assembly time.

LGTM

LGTM, but I would like to wait @asb ack about the unittest part.

LGTM, I think don't crash the compiler is higher priority.

• Check LocInfo rather than check ValVT.
• Also add an assertion to make sure no vector type got into wrong path.

@asb let you know psABI has merged the mapping symbol :)

• Drop UTC_ARGS: --version 2

LGTM, the version scheme of g already broken for a while.

Add comments for easier understand what's wrong for the current code gen.

LGTM :)

LGTM

Although Craig already give LGTM, so it's already ready to go, but I would like to give my blessing here, finally we could re-enable this, thanks you!

LGTM

Changes are look reasonable and good to me, but I am also new for ASan too, I guess we need few more eye to review.

Changes are look reasonable and good to me, but I am also new for ASan too, I guess we need few more eye to review.

Per my experience from RISC-V newlib port, LGTM.

LGTM, just a nit :)

In D144174#4134399, @arichardson wrote:

I am almost certain backtrace() does work on FreeBSD, so there should be a __linux__ check as well.
The comment says it only exists on glibc systems, but on FreeBSD libexecinfo provides this functionality.

Patch posted: https://reviews.llvm.org/D145164

RISC-V GCC has added -fasynchronous-unwind-tables to the default option for linux, maybe we should do that on clang side? this should able improve the situation.

I would say it should always set to true if I just read first paragraph, since we have softfloat for fp16.

I like the idea which decoupling the parsing logic of the normal march string parsing, and we can have a simple parser logic here since ELF attribute has specified that should have explicitly version number and separated by _.

LGTM

Do you mind provide your email address, so that I can put right author info in commit log?

LGTM, Verified with GCC and two clang w/ and w/o this patch, clang with this patch is matching GCC's behavior.

LGTM, my first impression is why so many diff, however I found it's because this patch also add llc -mtriple=riscv32/llc -mtriple=riscv64 without -mattr, and then...the diff algorithm result so many diff.

LGTM, I like this change, GCC also implement this way too, and it's more close to our RVV intrinsic programming model - abstract VL and VTYPE status from the front-end and middle-end.

LGTM, it's great to having a version number to distinguish different interface version

I am kind of conservative guy to deprecating those support - I will spend more times on figure out how to resolve that.

• Add testcase.

In D137417#4077226, @asb wrote:

In D137417#4076835, @kito-cheng wrote:

LGTM, this match what we have in psABI PR and binutils implemention, but I would like to make sure @asb is happy with this too :)

I've not done a super in-depth review, but from a high-level look it seems good to me. Despite the fact binutils already supports this, I'd probably rather see the psABI change approved before landing this patch. Is it possible that might happen soon?

In D139827#4080074, @luismarques wrote:

With the current (unpatched) LLVM tree on an RV64 SV39 host I get this ASAN test failure (for check-asan):

• AddressSanitizer-riscv64-linux :: TestCases/memcmp_test.cpp

With this patch on an RV64 SV48 host I get these test failures:

• AddressSanitizer-riscv64-linux :: TestCases/Linux/syscalls.cpp
• AddressSanitizer-riscv64-linux :: TestCases/Posix/bcmp_test.cpp
• AddressSanitizer-riscv64-linux :: TestCases/strncpy-overflow.cpp

I'll need to double-check if those results are actually correct but...
@kito-cheng Does that match your test results? If not, what do you observe? If it matches, have you investigated the new test failures?

LGTM

LGTM

LGTM

As a RISC-V GCC maintainer: GCC 14 is targeting on RVV intrinsic 1.0, but segment load store and rounding mode stuff might not meet the release, so the status will be pretty close to clang/LLVM (with @eopXD's changes, __riscv_ prefix and those simplification stuffs).

Agree with @jrtc27 about the test function name, we could keep the function name in the test file, that's also reduce lots of diff which is not really necessary.

I don't have a preference here, is any of riscv_RVV_CSR/ riscv_rvv_csr_type / __riscv_rvv_csr_t suitable?

LGTM, this match what we have in psABI PR and binutils implemention, but I would like to make sure @asb is happy with this too :)

LGTM

LGTM

Maybe multi-lib handling should split into another patch?

Could you split testcase update to a separated patch?

LGTM

LGTM for the dwarf number part.

Wait, I fell HasTailPolicy and HasMaskPolicy should be part of Policy object, and then it's the Omit to present it's no policy.

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM, the condition of those predictor function more reasonable now

LGTM

LGTM, this made the code more expressive.

LGTM with a nit, seems like it also improve the debug info (loc).

LGTM

LGTM

LGTM

LGTM, Ilike this improvement, it's further simplified the step of adding a new CPU info.