RKSimon added a comment.
LGTM as well
Repository:
rL LLVM
https://reviews.llvm.org/D41168
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334850: [X86] Lowering sqrt intrinsics to native IR
(authored by tkrupa, committed by ).
Herald added a subscriber:
craig.topper accepted this revision.
craig.topper added a comment.
LGTM
Repository:
rC Clang
https://reviews.llvm.org/D41168
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tkrupa updated this revision to Diff 151382.
tkrupa added a comment.
Fixed rounding mode calls.
Repository:
rC Clang
https://reviews.llvm.org/D41168
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/avx-builtins.c
test/CodeGen/avx512f-builtins.c
test/CodeGen/avx512vl-builtins.c
craig.topper added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:9222
+if (CC != 4)
+ return nullptr;
+Value *A = Builder.CreateExtractElement(Ops[0], (uint64_t)0);
What does returning nullptr here do?
Comment at:
tkrupa updated this revision to Diff 149693.
tkrupa added a comment.
Removed CHECK-NOTs for consistency.
Repository:
rC Clang
https://reviews.llvm.org/D41168
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/avx-builtins.c
test/CodeGen/avx512f-builtins.c
test/CodeGen/avx512vl-builtins.c
RKSimon added inline comments.
Comment at: test/CodeGen/sse2-builtins.c:1199
// CHECK-LABEL: test_mm_sqrt_sd
- // CHECK: call <2 x double> @llvm.x86.sse2.sqrt.sd(<2 x double> %{{.*}})
- // CHECK: extractelement <2 x double> %{{.*}}, i32 0
- // CHECK: insertelement <2 x
tkrupa updated this revision to Diff 149419.
tkrupa added a comment.
Added missing scalar intrinsics without rounding.
Repository:
rC Clang
https://reviews.llvm.org/D41168
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/avx-builtins.c
test/CodeGen/avx512f-builtins.c
RKSimon requested changes to this revision.
RKSimon added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D41168#1118624, @tkrupa wrote:
> Mask scalar case is closed and doesn't have any effects on this revision.
> Besides, I resolved issues connected to
tkrupa added a comment.
Mask scalar case is closed and doesn't have any effects on this revision.
Besides, I resolved issues connected to lowering scalar sqrt intrinsics without
rounding (that is, if https://reviews.llvm.org/D47621 is accepted). Should I
add them here to have everything sqrt
tkrupa added a comment.
I'll wait with upstreaming this patch until there's an agreement on mask scalar
approach.
Repository:
rC Clang
https://reviews.llvm.org/D41168
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.
LGTM
Repository:
rC Clang
https://reviews.llvm.org/D41168
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
tkrupa updated this revision to Diff 140937.
tkrupa added a comment.
I did the suggested corrections and improved the tests.
Repository:
rC Clang
https://reviews.llvm.org/D41168
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/avx-builtins.c
test/CodeGen/avx512f-builtins.c
tkrupa marked 4 inline comments as done.
tkrupa added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:8901
}
-
+ case X86::BI__builtin_ia32_sqrtsd_round_mask:
+ case X86::BI__builtin_ia32_sqrtss_round_mask: {
craig.topper wrote:
> What about
craig.topper added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:8901
}
-
+ case X86::BI__builtin_ia32_sqrtsd_round_mask:
+ case X86::BI__builtin_ia32_sqrtss_round_mask: {
What about __builtin_ia32_sqrtsd and __builtin_ia32_sqrtss?
Repository:
craig.topper added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:8904
+unsigned CC = cast(Ops[4])->getZExtValue();
+// Support only if the rounding mode is 4 (AKA CUR_DIRECTION), otherwise
keep the intrinsic.
+if (CC != 4) {
80 columns?
tkrupa updated this revision to Diff 140409.
tkrupa added a comment.
Removed renaming of the builtins (also in corresponding llvm patch).
Repository:
rC Clang
https://reviews.llvm.org/D41168
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/avx-builtins.c
test/CodeGen/avx512f-builtins.c
tkrupa commandeered this revision.
tkrupa added a reviewer: uriel.k.
tkrupa added a comment.
I was assigned to finish this task.
https://reviews.llvm.org/D41168
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
uriel.k updated this revision to Diff 128232.
uriel.k retitled this revision from "[X86][avx512] Lowering X86 avx512 sqrt
intrinsics to IR" to "[X86] Lowering X86 avx512 sqrt intrinsics to IR".
uriel.k edited the summary of this revision.
uriel.k added a comment.
now takes care of sqrt with
19 matches
Mail list logo
