LGTM. That's exactly what I meant to do when commenting in D156990.

Have you tried to directly modify some implementation in ExpandIntRes_FP_TO_{S,U}INT by adding another possibility of BF16_TO_FP when the FP type is TypeSoftPromoteHalf?

Can you add some cases without zfbfmin?

Maybe we are now confronted with a general issue. Are there any other operations that cannot be converted into f32 in front-end? I think you have noticed comparison and bf16-to-integer conversion.

LGTM.

In D150929#4545521, @craig.topper wrote:

This test crashes

float foo(__bf16 a, __bf16 b)  {
  return a + b;
}

error in backend: Cannot select: t35: f32,ch = load<(dereferenceable load (s16) from %ir.b.addr), anyext from bf16> t49, FrameIndex:i32<1>, undef:i32

Ping.

Since bf16 has been supported in backend, we can now come back to this clang patch. @craig.topper

Put the predicate both on the pseudo and the pattern.

Use 7 for rounding mode in testcases.

Adjust the spec version from 0.6 to 0.8

Relax the types in the type constraints for SDT_RISCVFMV_H_X and SDT_RISCV_FMV_X_EXTH

Move bf16 patterns into RISCVInstrInfoZfbfmin.td

Use the FPR16 register class for bf16 instructions

In D155418#4504837, @craig.topper wrote:

In D155418#4504816, @joshua-arch1 wrote:

Can you explain why it will be hard to maintain? The complexity of supporting Zfinx/Zdinx/Zhinx and all the special patterns for Zfhmin already seems way worse than passing an extra Valuetype to some classes.

I see. It seems that passing Valuetype to some codegen classes is the most effective way to address this issue if you want to use the same register class for different types. What we need to do now is to confirm that reusing FPR16 for bf16 is actually better than adding a new register class.

AArch64 also takes the approach of using a single register class for fp16 and bf16. Maybe there's some discussion around that decision we can find?

Can you explain why it will be hard to maintain? The complexity of supporting Zfinx/Zdinx/Zhinx and all the special patterns for Zfhmin already seems way worse than passing an extra Valuetype to some classes.

In D155418#4504763, @craig.topper wrote:

In D155418#4504759, @joshua-arch1 wrote:

Why are some patterns using FPR32 and FPR64 modified too?

Because they use the same classes in tablegen and I had to add a ValueType parameter to them.

Why are some patterns using FPR32 and FPR64 modified too?

Use predicate HasScalarHalfFPLoadStoreMove for patterns that f16 and bf16 share

In D153234#4493739, @asb wrote:

@joshua-arch1 are you still planning to work through the review feedback from Craig? Happy to help if useful.

In D152895#4448058, @hauhsu wrote:

Can lsan work for riscv64 now? I can run binaries compiled with -fsanitize=address adding ASAN_OPTIONS=detect_leaks=1, but no memory-leak error is reported.

Asan and Lsan should work on SV39 virtual memory mapping mode without applying this patchset.
There was an Asan/Lsan porting done by @EccoTheDolphin (https://reviews.llvm.org/D87580) and @luismarques (https://reviews.llvm.org/D92403).

Ping.

In D152498#4437085, @craig.topper wrote:

I don’t see tests for load or store or function arguments or returns.

Do the IR types need to be made legal in the backend.

In D152895#4439966, @vitalybuka wrote:

In D152895#4436941, @joshua-arch1 wrote:

Can lsan work for riscv64 now? I can run binaries compiled with -fsanitize=address adding ASAN_OPTIONS=detect_leaks=1, but no memory-leak error is reported.

This file applies to standalone lsan, without asan.

Can lsan work for riscv64 now? I can run binaries compiled with -fsanitize=address adding ASAN_OPTIONS=detect_leaks=1, but no memory-leak error is reported.

Rebase, as well as change the operand type in riscv_fpround_bf16/riscv_fpextend_bf16 from f16 to bf16.

To avoid crash in AArch64, ConstantFP nodes default to expand only in RISCV.

Ping.

The first version of this patch came in May 24th, earlier than a similar patch implemented by @asb https://reviews.llvm.org/D151663. Why could @asb's patch be accepted first?

1. Add argument IsBF to RVV_VECTOR_TYPE()
2. Add test for bf16 vectors in rvv-intrinsic-datatypes.cpp

In D152498#4407714, @craig.topper wrote:

No tests showing IR output?

Are you still working on this patch? I have implemented a similar one early in May 24th.
https://reviews.llvm.org/D151313

Update FP_TO_BF16 lowering by __truncsfbf2 libcall.

Modify implementation for f64 input.

Address bf16_to_fp/fp_to_bf16 via bitcast and shift.

Modify libcall for single-bfloat conversion.

In D151313#4367945, @asb wrote:

Sorry to not have specifics on this - I'm working on a patch in this same area and as you've found as well, it's quite fiddly. Using the __truncsfhf2 and __extendhfsf2 libcalls isn't correct - they expect 16-bit floats in a different format to bfloat16.

In D150929#4361151, @asb wrote:

I feel (IMHO) this is jumping to the endpoint a bit - the usual route for something like this is:

• MC layer support
• LLVM codegen support (and tests!)
• Any needed Clang support

I was hoping to push out some patches related to zfbfmin tests+codegen today but might time-out (if so, I'll return to it tomorrow).

I think the tentative conclusion of https://discourse.llvm.org/t/rfc-c-23-p1467r9-extended-floating-point-types-and-standard-names/70033 is to make __bf16_t an arithmetic type on all targets - I'd recommend keeping an eye on https://reviews.llvm.org/D150913 (and ideally helping to review it), as that's bound to interact with this patch.

Just like AArch64 and X86, I have updated my implementation by limiting bfloat16 only when Zfbfmin/Zvfbfmin/Zvfbfwma are enabled. Now the compiler can work for bfloat16 programs without crashing.

Hi, I'm adding support for __bf16 in clang, which will rely on FeatureStdExtZfbfmin. Can you merge this patch?

That doesn’t matter. The test I sent crashes, thats not acceptable.

Probably also means the ABI is not being implemented correctly.

In D150929#4355360, @craig.topper wrote:

In D150929#4355353, @joshua-arch1 wrote:

In D150929#4355303, @craig.topper wrote:

This test seems to crash in the backend with -march=rv64gc -mabi=lp64d

 __bf16 h1(__bf16 a) {
  return a;
}

I haven't done any backend support fo bf16 yet. What I want to do in this patch is only to enable __bf16 for clang.

We can’t have the compiler crashing if you use it. The backend has to go first.

In D150929#4355303, @craig.topper wrote:

This test seems to crash in the backend with -march=rv64gc -mabi=lp64d

 __bf16 h1(__bf16 a) {
  return a;
}

The RVI toolchain SIG is supposed to be setting up a task group to define intrinsics for all extensions.

In D147610#4294260, @craig.topper wrote:

In D147610#4294247, @joshua-arch1 wrote:

I'm wondering whether it is appropriate to just use FPR16 for the destination of fcvt.bf16.s? The destination is in BF16 format instead of simple FP16. Your implemention looks like just replacing fcvt.h.s with fcvt.bf16.s. Do we need to define a new register class?

Registers classes only distinguished by the registers in them and what their alignment and spill size are. It doesn't define anything about the format of the register. A BF16 specific register class would be identical in those properties to the FPR16 register class.

I'm wondering whether it is appropriate to just use FPR16 for the destination of fcvt.bf16.s? The destination is in BF16 format instead of simple FP16. Your implemention looks like just replacing fcvt.h.s with fcvt.bf16.s. Do we need to define a new register class?