[PATCH] Support for stack map generation in X86.

Andrew Trick atrick at apple.com
Tue Oct 22 23:45:12 PDT 2013


  Thanks for the review Hal.


================
Comment at: lib/CodeGen/StackMaps.cpp:99
@@ +98,3 @@
+  const MCSection *StackMapSection =
+    OutContext.getMachOSection("__LLVM_STACKMAPS", "__js_stackmaps", 0,
+                               SectionKind::getMetadata());
----------------
hfinkel at anl.gov wrote:
> Why is there a _js_ here? Why not just _llvm_stackmaps?
Oops, meant to fix that.

================
Comment at: test/CodeGen/X86/webkit-patchpoint.ll:36
@@ +35,3 @@
+
+attributes #0 = { noinline nounwind ssp uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf"="true" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
----------------
hfinkel at anl.gov wrote:
> Remove unneeded attributes.
I removed all the attributes.

================
Comment at: test/CodeGen/X86/webkit-stackmap.ll:142
@@ +141,3 @@
+;
+; FIXME: reenable these checks when stack maps support frame indices.
+; CHECK:        .long  8
----------------
hfinkel at anl.gov wrote:
> These look enabled?
Removed this FIXME.

================
Comment at: test/CodeGen/X86/webkit-stackmap.ll:160
@@ +159,3 @@
+  %add = add i64 %result, 3
+  ret i64 %add
+}
----------------
hfinkel at anl.gov wrote:
> This seems to be the only test where you actually capture the result of the patchpoint/stackmap; should we check the codegen? Also, if the results of these are dead, does that matter?
The codegen tests are in patchpoint.ll. I added a case there.

================
Comment at: test/CodeGen/X86/webkit-stackmap.ll:178
@@ +177,3 @@
+declare void @llvm.experimental.patchpoint.void(i32, i32, i8*, i32, ...) #2
+declare i64 @llvm.experimental.patchpoint.i64(i32, i32, i8*, i32, ...) #2
+
----------------
hfinkel at anl.gov wrote:
> The return types of these look different; does that matter?
There are two patchpoint intrinsics. One returns ia64, the other is void.


http://llvm-reviews.chandlerc.com/D1996



More information about the llvm-commits mailing list