[RS4GC] Strip noalias attribute after statepoint rewrite

[igor-laevsky]

Summary

We should remove noalias along with dereference and dereference_or_null attributes. Problem is that statepoint could potentially touch the entire heap including noalias objects. For example in this piece of code:

%obj = call noalias %jObject* new_instance(...)
store %val, %obj ; last use of %obj
call gc.statepoint()

Llvm will consider that %obj and statepoint call do not alias and move store after the statepoint.

[sanjoy]

include/llvm/IR/Instructions.h

1623 ↗

(On Diff #38279)

I'd prefer a name like isNoAlias, since doesNotAlias begs the question "does not alias what?".

And I think the does not alias other parameters bit is not accurate for return values.

lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

118 ↗

(On Diff #38279)

Nit: gc.statepoint

test/Transforms/RewriteStatepointsForGC/deref-pointers.ll

71 ↗

(On Diff #38279)

Can I ask you to please add these tests to the deopt-bundles directory as well? If that's too much work, please add a "TODO also test for noalias" in deopt-bundles/deref-pointers.ll; and I'll port the tests once they're in tree.

[igor-laevsky]

include/llvm/IR/Instructions.h

1623 ↗

(On Diff #38279)

I intentionally named this similar as the analogous Function method. Besides consistency it is also required for correctness - RemoveNonValidAttrAtIndex should work both on Function's and CallSite's.

In order to solve this I can rename both methods in a separate change. Or alternatively I can restructure code in RS4GC to not rely on similar naming in Function and CallSite.

Bit I am leaning toward leaving things as-is. Llvm lived for quite some time with doesNotAlias naming and probably people already used to it.

I have fixed comment in the updated diff.

[sanjoy]

include/llvm/IR/Instructions.h

1623 ↗

(On Diff #38442)

Bit I am leaning toward leaving things as-is. Llvm lived for quite some time with doesNotAlias naming and probably people already used to it.

SGTM. :)

llvm/trunk/include/llvm/IR/CallSite.h

#define CALLSITE_DELEGATE_SETTER(METHOD) \
   }
 
   /// @brief Extract the number of dereferenceable_or_null bytes for a call or
   /// parameter (0=unknown).
   uint64_t getDereferenceableOrNullBytes(uint16_t i) const {
     CALLSITE_DELEGATE_GETTER(getDereferenceableOrNullBytes(i));
   }
 
+  /// @brief Determine if the parameter or return value is marked with NoAlias
+  /// attribute.
+  /// @param n The parameter to check. 1 is the first parameter, 0 is the return
+  bool doesNotAlias(unsigned n) const {
+    CALLSITE_DELEGATE_GETTER(doesNotAlias(n));
+  }
+
   /// \brief Return true if the call should not be treated as a call to a
   /// builtin.
   bool isNoBuiltin() const {
     CALLSITE_DELEGATE_GETTER(isNoBuiltin());
   }
 
   /// @brief Return true if the call should not be inlined.
   bool isNoInline() const {

llvm/trunk/include/llvm/IR/Function.h

public:
 
   /// @brief Determine if the function returns a structure through first
   /// pointer argument.
   bool hasStructRetAttr() const {
     return AttributeSets.hasAttribute(1, Attribute::StructRet) ||
            AttributeSets.hasAttribute(2, Attribute::StructRet);
   }
 
-  /// @brief Determine if the parameter does not alias other parameters.
+  /// @brief Determine if the parameter or return value is marked with NoAlias
+  /// attribute.
   /// @param n The parameter to check. 1 is the first parameter, 0 is the return
   bool doesNotAlias(unsigned n) const {
     return AttributeSets.hasAttribute(n, Attribute::NoAlias);
   }
   void setDoesNotAlias(unsigned n) {
     addAttribute(n, Attribute::NoAlias);
   }
 

llvm/trunk/include/llvm/IR/Instructions.h

public:
   }
 
   /// \brief Extract the number of dereferenceable_or_null bytes for a call or
   /// parameter (0=unknown).
   uint64_t getDereferenceableOrNullBytes(unsigned i) const {
     return AttributeList.getDereferenceableOrNullBytes(i);
   }
 
+  /// @brief Determine if the parameter or return value is marked with NoAlias
+  /// attribute.
+  /// @param n The parameter to check. 1 is the first parameter, 0 is the return
+  bool doesNotAlias(unsigned n) const {
+    return AttributeList.hasAttribute(n, Attribute::NoAlias);
+  }
+
   /// \brief Return true if the call should not be treated as a call to a
   /// builtin.
   bool isNoBuiltin() const {
     return hasFnAttrImpl(Attribute::NoBuiltin) &&
       !hasFnAttrImpl(Attribute::Builtin);
   }
 
   /// \brief Return true if the call should not be inlined.
public:
   }
 
   /// \brief Extract the number of dereferenceable_or_null bytes for a call or
   /// parameter (0=unknown).
   uint64_t getDereferenceableOrNullBytes(unsigned i) const {
     return AttributeList.getDereferenceableOrNullBytes(i);
   }
 
+  /// @brief Determine if the parameter or return value is marked with NoAlias
+  /// attribute.
+  /// @param n The parameter to check. 1 is the first parameter, 0 is the return
+  bool doesNotAlias(unsigned n) const {
+    return AttributeList.hasAttribute(n, Attribute::NoAlias);
+  }
+
   /// \brief Return true if the call should not be treated as a call to a
   /// builtin.
   bool isNoBuiltin() const {
     // We assert in hasFnAttr if one passes in Attribute::NoBuiltin, so we have
     // to check it by hand.
     return hasFnAttrImpl(Attribute::NoBuiltin) &&
       !hasFnAttrImpl(Attribute::Builtin);
   }

llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

struct RewriteStatepointsForGC : public ModulePass {
 
   /// The IR fed into RewriteStatepointsForGC may have had attributes implying
   /// dereferenceability that are no longer valid/correct after
   /// RewriteStatepointsForGC has run.  This is because semantically, after
   /// RewriteStatepointsForGC runs, all calls to gc.statepoint "free" the entire
   /// heap.  stripNonValidAttributes (conservatively) restores correctness
   /// by erasing all attributes in the module that externally imply
   /// dereferenceability.
-  ///
+  /// Similar reasoning also applies to the noalias attributes. gc.statepoint
+  /// can touch the entire heap including noalias objects.
   void stripNonValidAttributes(Module &M);
 
   // Helpers for stripNonValidAttributes
   void stripNonValidAttributesFromBody(Function &F);
   void stripNonValidAttributesFromPrototype(Function &F);
 };
 } // namespace
 
static void RemoveNonValidAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH,
                                       unsigned Index) {
   AttrBuilder R;
   if (AH.getDereferenceableBytes(Index))
     R.addAttribute(Attribute::get(Ctx, Attribute::Dereferenceable,
                                   AH.getDereferenceableBytes(Index)));
   if (AH.getDereferenceableOrNullBytes(Index))
     R.addAttribute(Attribute::get(Ctx, Attribute::DereferenceableOrNull,
                                   AH.getDereferenceableOrNullBytes(Index)));
+  if (AH.doesNotAlias(Index))
+    R.addAttribute(Attribute::NoAlias);
 
   if (!R.empty())
     AH.setAttributes(AH.getAttributes().removeAttributes(
         Ctx, Index, AttributeSet::get(Ctx, Index, R)));
 }
 
 void
 RewriteStatepointsForGC::stripNonValidAttributesFromPrototype(Function &F) {

llvm/trunk/test/Transforms/RewriteStatepointsForGC/deopt-bundles/deref-pointers.ll

 ; RUN: opt -S -rewrite-statepoints-for-gc -rs4gc-use-deopt-bundles < %s | FileCheck %s
 
 ; CHECK: declare i8 addrspace(1)* @some_function_ret_deref()
 ; CHECK: define i8 addrspace(1)* @test_deref_arg(i8 addrspace(1)* %a)
 ; CHECK: define i8 addrspace(1)* @test_deref_or_null_arg(i8 addrspace(1)* %a)
+; CHECK: define i8 addrspace(1)* @test_noalias_arg(i8 addrspace(1)* %a)
 
 declare void @foo()
 
 declare i8 addrspace(1)* @some_function() "gc-leaf-function"
 
 declare void @some_function_consumer(i8 addrspace(1)*) "gc-leaf-function"
 
 declare dereferenceable(4) i8 addrspace(1)* @some_function_ret_deref() "gc-leaf-function"
+declare noalias i8 addrspace(1)* @some_function_ret_noalias() "gc-leaf-function"
 
 define i8 addrspace(1)* @test_deref_arg(i8 addrspace(1)* dereferenceable(4) %a) gc "statepoint-example" {
 entry:
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %a
 }
 
 define i8 addrspace(1)* @test_deref_or_null_arg(i8 addrspace(1)* dereferenceable_or_null(4) %a) gc "statepoint-example" {
 entry:
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %a
 }
 
+define i8 addrspace(1)* @test_noalias_arg(i8 addrspace(1)* noalias %a) gc "statepoint-example" {
+entry:
+  call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
+  ret i8 addrspace(1)* %a
+}
+
 define i8 addrspace(1)* @test_deref_retval() gc "statepoint-example" {
 ; CHECK-LABEL: @test_deref_retval(
 ; CHECK: %a = call i8 addrspace(1)* @some_function()
 entry:
   %a = call dereferenceable(4) i8 addrspace(1)* @some_function()
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %a
 }
 
 define i8 addrspace(1)* @test_deref_or_null_retval() gc "statepoint-example" {
 ; CHECK-LABEL: @test_deref_or_null_retval(
 ; CHECK: %a = call i8 addrspace(1)* @some_function()
 entry:
   %a = call dereferenceable_or_null(4) i8 addrspace(1)* @some_function()
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %a
 }
 
+define i8 addrspace(1)* @test_noalias_retval() gc "statepoint-example" {
+; CHECK-LABEL: @test_noalias_retval(
+; CHECK: %a = call i8 addrspace(1)* @some_function()
+entry:
+  %a = call noalias i8 addrspace(1)* @some_function()
+  call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
+  ret i8 addrspace(1)* %a
+}
+
 define i8 @test_md(i8 addrspace(1)* %ptr) gc "statepoint-example" {
 ; CHECK-LABEL: @test_md(
 ; CHECK: %tmp = load i8, i8 addrspace(1)* %ptr, !tbaa !0
 entry:
   %tmp = load i8, i8 addrspace(1)* %ptr, !tbaa !0
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 %tmp
 }
 
 define i8 addrspace(1)* @test_decl_only_attribute(i8 addrspace(1)* %ptr) gc "statepoint-example" {
 ; CHECK-LABEL: @test_decl_only_attribute(
 ; No change here, but the prototype of some_function_ret_deref should have changed.
 ; CHECK: call i8 addrspace(1)* @some_function_ret_deref()
 entry:
   %a = call i8 addrspace(1)* @some_function_ret_deref()
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %a
 }
 
+define i8 addrspace(1)* @test_decl_only_noalias(i8 addrspace(1)* %ptr) gc "statepoint-example" {
+; CHECK-LABEL: @test_decl_only_noalias(
+; No change here, but the prototype of some_function_ret_noalias should have changed.
+; CHECK: call i8 addrspace(1)* @some_function_ret_noalias()
+entry:
+  %a = call i8 addrspace(1)* @some_function_ret_noalias()
+  call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
+  ret i8 addrspace(1)* %a
+}
+
 define i8 addrspace(1)* @test_callsite_arg_attribute(i8 addrspace(1)* %ptr) gc "statepoint-example" {
 ; CHECK-LABEL: @test_callsite_arg_attribute(
 ; CHECK: call void @some_function_consumer(i8 addrspace(1)* %ptr)
 ; CHECK: !0 = !{!1, !1, i64 0}
 ; CHECK: !1 = !{!"red", !2}
 ; CHECK: !2 = !{!"blue"}
 entry:
-  call void @some_function_consumer(i8 addrspace(1)* dereferenceable(4) %ptr)
+  call void @some_function_consumer(i8 addrspace(1)* dereferenceable(4) noalias %ptr)
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %ptr
 }
 !0 = !{!1, !1, i64 0, i64 1}
 !1 = !{!"red", !2}
 !2 = !{!"blue"}

llvm/trunk/test/Transforms/RewriteStatepointsForGC/deref-pointers.ll

 ; RUN: opt -S -rewrite-statepoints-for-gc < %s | FileCheck %s
 
 declare void @foo()
 declare i8 addrspace(1)* @some_function()
 declare void @some_function_consumer(i8 addrspace(1)*)
 declare dereferenceable(4) i8 addrspace(1)* @some_function_ret_deref()
 ; CHECK: declare i8 addrspace(1)* @some_function_ret_deref()
+declare noalias i8 addrspace(1)* @some_function_ret_noalias()
+; CHECK: declare i8 addrspace(1)* @some_function_ret_noalias()
 
 define i8 addrspace(1)* @test_deref_arg(i8 addrspace(1)* dereferenceable(4) %a) gc "statepoint-example" {
 ; CHECK: define i8 addrspace(1)* @test_deref_arg(i8 addrspace(1)* %a)
 entry:
   call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
   ret i8 addrspace(1)* %a
 }
 

 ; CHECK-LABEL: @test_callsite_arg_attribute(
 entry:
 ; CHECK: call void @some_function_consumer(i8 addrspace(1)* %ptr)
   call void @some_function_consumer(i8 addrspace(1)* dereferenceable(4) %ptr)
   call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
   ret i8 addrspace(1)* %ptr
 }
 
+define i8 addrspace(1)* @test_noalias_arg(i8 addrspace(1)* noalias %a) gc "statepoint-example" {
+; CHECK: define i8 addrspace(1)* @test_noalias_arg(i8 addrspace(1)* %a)
+entry:
+  call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
+  ret i8 addrspace(1)* %a
+}
+
+define i8 addrspace(1)* @test_noalias_retval() gc "statepoint-example" {
+; CHECK-LABEL: @test_noalias_retval(
+entry:
+  %a = call noalias i8 addrspace(1)* @some_function()
+; CHECK: %a = call i8 addrspace(1)* @some_function()
+  call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
+  ret i8 addrspace(1)* %a
+}
+
+define i8 addrspace(1)* @test_decl_only_noalias(i8 addrspace(1)* %ptr) gc "statepoint-example" {
+; CHECK-LABEL: @test_decl_only_noalias(
+entry:
+; No change here, but the prototype of some_function_ret_noalias should have changed.
+; CHECK: call i8 addrspace(1)* @some_function_ret_noalias()
+  %a = call i8 addrspace(1)* @some_function_ret_noalias()
+  call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
+  ret i8 addrspace(1)* %a
+}
+
+define i8 addrspace(1)* @test_callsite_arg_noalias(i8 addrspace(1)* %ptr) gc "statepoint-example" {
+; CHECK-LABEL: @test_callsite_arg_noalias(
+entry:
+; CHECK: call void @some_function_consumer(i8 addrspace(1)* %ptr)
+  call void @some_function_consumer(i8 addrspace(1)* noalias %ptr)
+  call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0)
+  ret i8 addrspace(1)* %ptr
+}
+
 declare i32 @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
 
 !0 = !{!1, !1, i64 0, i64 1}
 !1 = !{!"red", !2}
 !2 = !{!"blue"}
 
 ; CHECK: !0 = !{!1, !1, i64 0}
 ; CHECK: !1 = !{!"red", !2}
 ; CHECK: !2 = !{!"blue"}