LGTM

Or even use clang to generate LLVM IR ahead of time...

This may not be a good idea for the feature of _BitInt() , which aims to offering more optimization opportunities for IR like i256, i257, i512... @erichkeane

LGTM. Will you update the backend libcall?

To be honest, I'm not familiar in compiler-rt component. And my experience before was mainly to find out the feasibility to support arbitrary-bits-division for _BitInt(). Fortunately, my proposal was basically same as this one: emit a new libcall in backend; add library support; use malloc()... And I even used a more simple division algorithm model: sub-shift, which has been shared in the RFC link. So my experience before may not be very helpful here. Anyway I can try my best. So everyone is OK with use malloc(), right? That was my biggest concern. BTW FYI the largest _BitInt() support is 8,388,608 . Hi @mgehre-amd Will you upload another patch to set the bitint.a as a default link library of clang when this patch gets landed in? I believe FE will reopen _BitInt() > 128 bit unless division is supported by default but not by option --rtlib=compiler-rt. @erichkeane @aaron.ballman

All right. I'll commit then.

In D115850#3223925, @craig.topper wrote:

@FreddyYe do you mind taking this back to commit since the code change is yours.

Thank you both! Your speculating is right! That's the original motivation for this patch.

ping...

Why would TLI be involved in that test. Isn't the sqrt call converted to llvm.sqrt by the frontend?

For example this patch can fix such performance drop on Linux:

CodeGenPasses is a legacy pass manager here. For now. llc.cpp has not switched to the new pass manager. llc.cpp does add this pass. And many passes in codegen need a right TargetLibraryInfo. If not adding it here with the module's triple info. TargetLibraryInfo will be initialized with a null string. That results in setting many library calls unavailable, as conservative as possible. I think it's not reasonable. Welcome discuss! Welcome adding more reviewers.

agree with @pengfei . sorry for noise.

clang-format

changed into unsigned short

convertion -> conversion.

After reading the whole logic, I'll judge this as not a defect. Since TypePromotion::isSupportedValue() has checked before codes here. ReturnInst's operand won't be null. So getReturnValue() here won't return Null. I'll close this PR, sorry for the noise.

Ping.

address comments. add alias for 36 intrinsics in all.

In D112777#3114566, @craig.topper wrote:

In D112777#3114560, @FreddyYe wrote:

In D112777#3114502, @craig.topper wrote:

Not directly related to this patch, but why is the suffix _pch and _sch when the instruction names end in CPH and CSH? It kind of seems like the correct intrinsic name would have been _mm_fmulc_ph.

Why does the name here need to be aligned with mul_ps/pd? This a "complex" multiply which is a different operation. Is gcc also going to add aliases?

I can answer the second question. The prefix "f" can be judged as the mnemonic to distinguish fma instrinsics. The suffix "c" can be judged as the mnemonic of "complex". So add "f" mnemonic in this multiply intrinsics is ambiguous. gcc will add aliases, too. The first question is also a good question. But for now, it's not very conflict to old intrinsics, I think.

But the mnemonic for the instructions here do include an 'F'. While the mulps/mulpd instruction names do not include an 'F'. So we're creating new intrinsics that are further away from the mnemonics of the instructions. Is your argument that the instruction mnemonics are wrong and shouldn't include an 'F'?

Thx for review. I found I missed many intrinsics, including _fcmul_* series. they need to be aliased with cmul_*. pls wait for update.

In D112777#3114502, @craig.topper wrote:

Not directly related to this patch, but why is the suffix _pch and _sch when the instruction names end in CPH and CSH? It kind of seems like the correct intrinsic name would have been _mm_fmulc_ph.

Why does the name here need to be aligned with mul_ps/pd? This a "complex" multiply which is a different operation. Is gcc also going to add aliases?

Ping.

changed into assert()

Besides, I didn't find difference between StoreInst and ReturnInst, why only warn here?

The difference is Store->getValueOperand() doesn't return a nullptr but Return->getValueOperand() does.

clang-formats.

In D112777#3095847, @pengfei wrote:

*_mul_pch is to align with *_mul_ps annd *_mul_pd

And *_mul_ph?

In D111778#3063891, @erichkeane wrote:

Supporting these types requires changes to the library so that we can check these, right? Do you have the library changes for these as well? The constants used to check for these features/etc needs to be present in the glibc library.

Yes. It's highly related to the ProcessorFeatures I mentioned above. Does the library changes refer to the compiler-rt/cpu_model.c? I think that change depends on libgcc. For now libgcc has no strong will to update that table.

As mentioned before:

Address comments.

Address part of comments. Thanks for review.

Address comments.

Address comments.

add {{$}}. add comments at first line.

Sorry for late response. Thanks for your comments, @DavidSpickett !

In principle I agree but did you have this failure mode actually happen?

No failure happens for now, but may happen in the future if we continue to use -SAME. Pls read the example I gave in last comment.

I found the old way cannot verify if there are some extra outputs between two different CHECK-SAME. So I changed to CHECK-NEXT. But it will introduce bad format issue. Anyway, the old way has broken clang-format already. So I would prefer the CHECK-NEXT. WDYT?

clean test.

This patch is to align with gcc's behavior on __builtin_thread_pointer () on Linux. Example output of gcc: https://godbolt.org/z/9oTTW53qe
Related bugzilla of gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96200, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96955

LGTM.

LGTM.

Refine tests.

nfc refine.

Thanks for review! Addressed comments.

Example reproduce: https://gcc.godbolt.org/z/czrY74sxv