kristof.beyls added a comment.
This intrinsic got added to gcc a while ago and should become available in the
upcoming gcc 9 release.
In gcc however, the prototype of the intrinsic is slightly different (see
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html):
type __builtin_speculation_safe_value (type val, type failval)
It provides a second optional argument "failval". From the gcc documentation:
"The function may use target-dependent speculation tracking state to cause
failval to be returned when it is known that speculative execution has
incorrectly predicted a conditional branch operation."
So, when implementing the intrinsic using a speculation barrier such as lfence,
that failval argument doesn't have any effect. However, when lowering the
intrinsic using speculation tracking similar to how that's used in SLH, this
failval parameter is used to return a non-zero value on miss-speculation, in
case the developer prefers that over the default zero value.
I think we should make the intrinsic compatible with the one introduced in gcc.
================
Comment at: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll:1
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
--check-prefix=X64
+
----------------
I guess the -mtriple command line option may not be needed since the IR file
contain "target triple" and "target datalayout" information?
================
Comment at: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll:3-4
+
+; ModuleID = 'hello.cpp'
+source_filename = "hello.cpp"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
----------------
I guess this is not strictly necessary for this test, so should be removed?
================
Comment at: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll:8-62
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo32i(i32 %a) #0 {
+entry:
+ %a.addr = alloca i32, align 4
+ %b = alloca i32, align 4
+ %b_safe = alloca i32, align 4
+ %c = alloca i32, align 4
----------------
Thanks for those updates, Zola. It makes it easier to compare this patch with
the code I wrote earlier.
Doing that comparison, I see that I had a few changes too in target-independent
SelectionDAG under lib/Codegen/SelectionDAG.
IIRC, you might find that you'll need that code if you also add tests here to
test the correct thing happens when applying the intrinsic on other types than
i32 or i64.
You probably also would want a test on a pointer data type here, I guess.
================
Comment at: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll:64-71
+attributes #0 = { noinline nounwind optnone uwtable
"correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false"
"less-precise-fpmad"="false" "min-legal-vector-width"="0"
"no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf"
"no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false"
"no-signed-zeros-fp-math"="false" "no-trapping-math"="false"
"stack-protector-buffer-size"="8" "target-cpu"="x86-64"
"target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false"
"use-soft-float"="false" }
+attributes #1 = { nounwind }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
----------------
I guess this is not strictly necessary for this test, so should be removed?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59827/new/
https://reviews.llvm.org/D59827
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
