Add InaccessibleMemOnly and inaccessibleMemOrArgMemOnly attributes

[vaivaswatha]

Summary

This patch introduces two new function attributes

InaccessibleMemOnly: This attribute indicates that the function may only access memory that is not accessible by the program/IR being compiled. This is a weaker form of ReadNone.
inaccessibleMemOrArgMemOnly: This attribute indicates that the function may only access memory that is either not accessible by the program/IR being compiled, or is pointed to by its pointer arguments. This is a weaker form of ArgMemOnly

Test cases have been updated. This revision uses this (https://github.com/llvm-mirror/llvm/commit/d001932f3a8aa1ebd1555162fdce365f011bc292) as reference.

[jmolloy]

Hi Vaivaswatha,

Thanks for pushing this forward. Firstly, this patch should be split into at about four separate patches:

1. Introduce new function attributes
2. Modify GlobalsAA
3. Add these new attributes to libc functions
4. Add __mulsc3

The LLVM community prefer small, targetted patches, and each should come with at least one test case.

There will be a lot of bikeshedding on the names for these attributes. I'll just throw my penniworth in the ring and say I don't like the current names. "almostreadnone" only tells me that it isn't readnone - I'm going to have to look at the LangRef (which needs updating as part of patch (1) by the way) to see what it means.

A well-named attribute should imply its semantics at least vaguely. "readnone" for example is self explanatory. It'd be good to replace "almost" with something that describes how it differs from "readnone".

I'd suggest having a look the commits that introduced other attributes (like "norecurse" that I added recently) - they will show you what tests need touching/adding. Adding an attribute requires more tests than a normal commit, because you change the bitcode format and we need to ensure we don't regress this.

Did you run the LLVM regression tests? I think this line:

ATTR_KIND_ALMOST_ARGMEM_ONLY = 50,

should have broken at least one test - there is an invalid bitcode test that uses "50" as a known-invalid attribute kind.

/// @brief Determine if the function may read only non IR visible globals,

What about heap allocations or any other allocation that is considered an object, like an alloca whose address has been taken? Are we restricting this completely to globals, or do we need to expand this to cover all addressable objects?

Cheers,

James

[mehdi_amini]

I don't see changes to the LangRef?

include/llvm/IR/Attributes.td

23

Can you clarify in the description that ReadNone is stricter that AlmostReadNone (i.e. one contains the other)? (if I understood correctly)

29

Again if we could make very clear and explicit the relationship between AlmostArgMemOnly and ArgMemOnly in the description.

include/llvm/IR/Function.h

296

@brief unneeded here.

[vaivaswatha]

In D15499#309992, @jmolloy wrote:

Hi Vaivaswatha,

Thanks for pushing this forward. Firstly, this patch should be split into at about four separate patches:

1. Introduce new function attributes
2. Modify GlobalsAA
3. Add these new attributes to libc functions
4. Add __mulsc3

The LLVM community prefer small, targetted patches, and each should come with at least one test case.

I can do that. So the first patch can be part of this review and I'll create new ones as I go along?

There will be a lot of bikeshedding on the names for these attributes. I'll just throw my penniworth in the ring and say I don't like the current names. "almostreadnone" only tells me that it isn't readnone - I'm going to have to look at the LangRef (which needs updating as part of patch (1) by the way) to see what it means.

A well-named attribute should imply its semantics at least vaguely. "readnone" for example is self explanatory. It'd be good to replace "almost" with something that describes how it differs from "readnone".

I'd suggest having a look the commits that introduced other attributes (like "norecurse" that I added recently) - they will show you what tests need touching/adding. Adding an attribute requires more tests than a normal commit, because you change the bitcode format and we need to ensure we don't regress this.

Did you run the LLVM regression tests? I think this line:

ATTR_KIND_ALMOST_ARGMEM_ONLY = 50,

should have broken at least one test - there is an invalid bitcode test that uses "50" as a known-invalid attribute kind.

I did run the regression tests and as expected saw some failures. I am currently working on them. I'll update this patch with the corrected tests, and only covering work-item (1). Will submit more patches as it progresses.

/// @brief Determine if the function may read only non IR visible globals,

What about heap allocations or any other allocation that is considered an object, like an alloca whose address has been taken? Are we restricting this completely to globals, or do we need to expand this to cover all addressable objects?

Does "read only non-IR visible memory" sound more appropriate ?

Cheers,

James

I'll also update the langref when I submit the new patch.

[vaivaswatha]

Changes as per reviews: This change now just adds the two attributes and relevant tests.

[hfinkel]

We should say module instead of program/IR, but otherwise LGTM.

docs/LangRef.rst

1248 ↗

(On Diff #42976)

program/IR -> module

1252 ↗

(On Diff #42976)

program/IR -> module

[deadalnix]

What is the intended use case of that attribute ? Also, what is the intended effect when module are merged ?

include/llvm/Analysis/TargetLibraryInfo.def

 TLI_DEFINE_ENUM_INTERNAL(memcpy_chk)
 TLI_DEFINE_STRING_INTERNAL("__memcpy_chk")
 /// void *__memmove_chk(void *s1, const void *s2, size_t n, size_t s1size);
 TLI_DEFINE_ENUM_INTERNAL(memmove_chk)
 TLI_DEFINE_STRING_INTERNAL("__memmove_chk")
 /// void *__memset_chk(void *s, char v, size_t n, size_t s1size);
 TLI_DEFINE_ENUM_INTERNAL(memset_chk)
 TLI_DEFINE_STRING_INTERNAL("__memset_chk")
+//  float _Complex __mulsc3(float __a, float __b, float __c, float __d)
+TLI_DEFINE_ENUM_INTERNAL(mulsc3)
+TLI_DEFINE_STRING_INTERNAL("__mulsc3")
 /// double __sincospi_stret(double x);
 TLI_DEFINE_ENUM_INTERNAL(sincospi_stret)
 TLI_DEFINE_STRING_INTERNAL("__sincospi_stret")
 /// float __sincospif_stret(float x);
 TLI_DEFINE_ENUM_INTERNAL(sincospif_stret)
 TLI_DEFINE_STRING_INTERNAL("__sincospif_stret")
 /// double __sinpi(double x);
 TLI_DEFINE_ENUM_INTERNAL(sinpi)

 TLI_DEFINE_ENUM_INTERNAL(coshl)
 TLI_DEFINE_STRING_INTERNAL("coshl")
 /// long double cosl(long double x);
 TLI_DEFINE_ENUM_INTERNAL(cosl)
 TLI_DEFINE_STRING_INTERNAL("cosl")
 /// char *ctermid(char *s);
 TLI_DEFINE_ENUM_INTERNAL(ctermid)
 TLI_DEFINE_STRING_INTERNAL("ctermid")
+//  void exit(int status);
+TLI_DEFINE_ENUM_INTERNAL(exit)
+TLI_DEFINE_STRING_INTERNAL("exit")
 /// double exp(double x);
 TLI_DEFINE_ENUM_INTERNAL(exp)
 TLI_DEFINE_STRING_INTERNAL("exp")
 /// double exp10(double x);
 TLI_DEFINE_ENUM_INTERNAL(exp10)
 TLI_DEFINE_STRING_INTERNAL("exp10")
 /// float exp10f(float x);
 TLI_DEFINE_ENUM_INTERNAL(exp10f)

include/llvm/Bitcode/LLVMBitCodes.h

enum AttributeKindCodes {
     ATTR_KIND_JUMP_TABLE = 40,
     ATTR_KIND_DEREFERENCEABLE = 41,
     ATTR_KIND_DEREFERENCEABLE_OR_NULL = 42,
     ATTR_KIND_CONVERGENT = 43,
     ATTR_KIND_SAFESTACK = 44,
     ATTR_KIND_ARGMEMONLY = 45,
     ATTR_KIND_SWIFT_SELF = 46,
     ATTR_KIND_SWIFT_ERROR = 47,
-    ATTR_KIND_NO_RECURSE = 48
+    ATTR_KIND_NO_RECURSE = 48,
+    ATTR_KIND_ALMOST_READ_NONE = 49,
+    ATTR_KIND_ALMOST_ARGMEM_ONLY = 50,
   };
 
   enum ComdatSelectionKindCodes {
     COMDAT_SELECTION_KIND_ANY = 1,
     COMDAT_SELECTION_KIND_EXACT_MATCH = 2,
     COMDAT_SELECTION_KIND_LARGEST = 3,
     COMDAT_SELECTION_KIND_NO_DUPLICATES = 4,
     COMDAT_SELECTION_KIND_SAME_SIZE = 5,
   };
 
 } // End bitc namespace
 } // End llvm namespace
 
 #endif

include/llvm/IR/Attributes.td

 class StrBoolAttr<string S> : Attr<S>;
 
 /// Target-independent enum attributes.
 
 /// Alignment of parameter (5 bits) stored as log2 of alignment with +1 bias.
 /// 0 means unaligned (different from align(1)).
 def Alignment : EnumAttr<"align">;
 
+/// Function might access globals, but none of these globals can alias with 
+/// any memory location accessible from the IR being optimized (example - 
+/// internal states of libraries). Function does not access any memory
+/// that is accessible from the IR.
+def AlmostReadNone : EnumAttr<"almostreadnone">;
+
+/// Function might access globals, but none of these globals can alias with 
+/// any memory location accessible from the IR being optimized (example - 
+/// internal states of libraries). It may access memory pointed to by 
+/// its arguments.
+def AlmostArgMemOnly : EnumAttr<"almostargmemonly">;
+
 /// inline=always.
 def AlwaysInline : EnumAttr<"alwaysinline">;
 
 /// Funciton can access memory only using pointers based on its arguments.
 def ArgMemOnly : EnumAttr<"argmemonly">;
 
 /// Callee is recognized as a builtin, despite nobuiltin attribute on its
 /// declaration.

include/llvm/IR/Function.h

public:
   /// @brief Determine if the call can access memmory only using pointers based
   /// on its arguments.
   bool onlyAccessesArgMemory() const {
     return AttributeSets.hasAttribute(AttributeSet::FunctionIndex,
                                       Attribute::ArgMemOnly);
   }
   void setOnlyAccessesArgMemory() { addFnAttr(Attribute::ArgMemOnly); }
 
+  /// @brief Determine if the function may read only non IR visible globals, 
+  /// and does not access any IR visible globals.
+  bool isAlmostReadNone() const {
+    return AttributeSets.hasAttribute(AttributeSet::FunctionIndex,
+                                      Attribute::AlmostReadNone);
+  }
+  void setAlmostReadNone() { addFnAttr(Attribute::AlmostReadNone); }
+
+  /// @brief Determine if the function may read only non IR visible globals,
+  /// or only memory pointed to by pointer arguments.
+  bool isAlmostArgMemOnly() const {
+    return AttributeSets.hasAttribute(AttributeSet::FunctionIndex,
+                                      Attribute::AlmostArgMemOnly);
+  }
+  void setAlmostArgMemOnly() { addFnAttr(Attribute::AlmostArgMemOnly); }
+
   /// @brief Determine if the function cannot return.
   bool doesNotReturn() const {
     return AttributeSets.hasAttribute(AttributeSet::FunctionIndex,
                                       Attribute::NoReturn);
   }
   void setDoesNotReturn() {
     addFnAttr(Attribute::NoReturn);
   }

lib/Analysis/GlobalsModRef.cpp

for (unsigned i = 0, e = SCC.size(); i != e && !KnowNothing; ++i) {
       Function *F = SCC[i]->getFunction();
       if (!F) {
         KnowNothing = true;
         break;
       }
 
       if (F->isDeclaration()) {
         // Try to get mod/ref behaviour from function attributes.
-        if (F->doesNotAccessMemory()) {
+        if (F->doesNotAccessMemory() || F->isAlmostReadNone() ||
+            F->onlyAccessesArgMemory() || F->isAlmostArgMemOnly()) {
           // Can't do better than that!
         } else if (F->onlyReadsMemory()) {
           FI.addModRefInfo(MRI_Ref);
           if (!F->isIntrinsic())
             // This function might call back into the module and read a global -
             // consider every global as possibly being read by this function.
             FI.setMayReadAnyGlobal();
         } else {

lib/Bitcode/Writer/BitcodeWriter.cpp

static void WriteStringRecord(unsigned Code, StringRef Str,
   // Emit the finished record.
   Stream.EmitRecord(Code, Vals, AbbrevToUse);
 }
 
 static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
   switch (Kind) {
   case Attribute::Alignment:
     return bitc::ATTR_KIND_ALIGNMENT;
+  case Attribute::AlmostReadNone:
+    return bitc::ATTR_KIND_ALMOST_READ_NONE;
+  case Attribute::AlmostArgMemOnly:
+    return bitc::ATTR_KIND_ALMOST_ARGMEM_ONLY;
   case Attribute::AlwaysInline:
     return bitc::ATTR_KIND_ALWAYS_INLINE;
   case Attribute::ArgMemOnly:
     return bitc::ATTR_KIND_ARGMEMONLY;
   case Attribute::Builtin:
     return bitc::ATTR_KIND_BUILTIN;
   case Attribute::ByVal:
     return bitc::ATTR_KIND_BY_VAL;

lib/IR/Attributes.cpp

uint64_t AttributeImpl::getAttrMask(Attribute::AttrKind Val) {
   case Attribute::Builtin:         return 1ULL << 41;
   case Attribute::OptimizeNone:    return 1ULL << 42;
   case Attribute::InAlloca:        return 1ULL << 43;
   case Attribute::NonNull:         return 1ULL << 44;
   case Attribute::JumpTable:       return 1ULL << 45;
   case Attribute::Convergent:      return 1ULL << 46;
   case Attribute::SafeStack:       return 1ULL << 47;
   case Attribute::NoRecurse:       return 1ULL << 48;
+  case Attribute::AlmostReadNone:  return 1ULL << 49;
+  case Attribute::AlmostArgMemOnly: return 1ULL << 50;
   case Attribute::Dereferenceable:
     llvm_unreachable("dereferenceable attribute not supported in raw format");
     break;
   case Attribute::DereferenceableOrNull:
     llvm_unreachable("dereferenceable_or_null attribute not supported in raw "
                      "format");
     break;
   case Attribute::ArgMemOnly:

lib/Transforms/IPO/FunctionAttrs.cpp

struct FunctionAttrs : public CallGraphSCCPass {
   }
 
   bool runOnSCC(CallGraphSCC &SCC) override;
   bool doInitialization(CallGraph &CG) override {
     Revisit.clear();
     return false;
   }
   bool doFinalization(CallGraph &CG) override;
-  
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesCFG();
     AU.addRequired<AssumptionCacheTracker>();
     AU.addRequired<TargetLibraryInfoWrapperPass>();
     CallGraphSCCPass::getAnalysisUsage(AU);
   }
 
 private:

 
 static void setOnlyReadsMemory(Function &F) {
   if (!F.onlyReadsMemory()) {
     F.setOnlyReadsMemory();
     ++NumAnnotated;
   }
 }
 
+static void setAlmostReadNone(Function &F) {
+  if (!F.isAlmostReadNone()) {
+    F.setAlmostReadNone();
+    ++NumAnnotated;
+  }
+}
+
+static void setAlmostArgMemOnly(Function &F) {
+  if (!F.isAlmostArgMemOnly()) {
+    F.setAlmostArgMemOnly();
+    ++NumAnnotated;
+  }
+}
+
 static void setDoesNotThrow(Function &F) {
   if (!F.doesNotThrow()) {
     F.setDoesNotThrow();
     ++NumAnnotated;
   }
 }
 
 static void setDoesNotCapture(Function &F, unsigned n) {
case LibFunc::system:
     setDoesNotCapture(F, 1);
     setOnlyReadsMemory(F, 1);
     break;
   case LibFunc::malloc:
     if (FTy->getNumParams() != 1 || !FTy->getReturnType()->isPointerTy())
       return false;
     setDoesNotThrow(F);
     setDoesNotAlias(F, 0);
+    setAlmostReadNone(F);
     break;
   case LibFunc::memcmp:
     if (FTy->getNumParams() != 3 || !FTy->getParamType(0)->isPointerTy() ||
         !FTy->getParamType(1)->isPointerTy())
       return false;
     setOnlyReadsMemory(F);
     setDoesNotThrow(F);
     setDoesNotCapture(F, 1);
case LibFunc::mktime:
     break;
   case LibFunc::realloc:
     if (FTy->getNumParams() != 2 || !FTy->getParamType(0)->isPointerTy() ||
         !FTy->getReturnType()->isPointerTy())
       return false;
     setDoesNotThrow(F);
     setDoesNotAlias(F, 0);
     setDoesNotCapture(F, 1);
+    setAlmostArgMemOnly(F);
     break;
   case LibFunc::read:
     if (FTy->getNumParams() != 3 || !FTy->getParamType(1)->isPointerTy())
       return false;
     // May throw; "read" is a valid pthread cancellation point.
     setDoesNotCapture(F, 2);
     break;
   case LibFunc::rewind:
case LibFunc::bzero:
     setDoesNotThrow(F);
     setDoesNotCapture(F, 1);
     break;
   case LibFunc::calloc:
     if (FTy->getNumParams() != 2 || !FTy->getReturnType()->isPointerTy())
       return false;
     setDoesNotThrow(F);
     setDoesNotAlias(F, 0);
+    setAlmostReadNone(F);
     break;
   case LibFunc::chmod:
   case LibFunc::chown:
     if (FTy->getNumParams() == 0 || !FTy->getParamType(0)->isPointerTy())
       return false;
     setDoesNotThrow(F);
     setDoesNotCapture(F, 1);
     setOnlyReadsMemory(F, 1);
static bool inferPrototypeAttributes(Function &F, const TargetLibraryInfo &TLI) {
   case LibFunc::fsetpos:
   case LibFunc::flockfile:
   case LibFunc::funlockfile:
   case LibFunc::ftrylockfile:
     if (FTy->getNumParams() == 0 || !FTy->getParamType(0)->isPointerTy())
       return false;
     setDoesNotThrow(F);
     setDoesNotCapture(F, 1);
+    if (TheLibFunc == LibFunc::free) {
+      // TODO: Perhaps others can be set too.
+      setAlmostArgMemOnly(F);
+    }
     break;
   case LibFunc::ferror:
     if (FTy->getNumParams() != 1 || !FTy->getParamType(0)->isPointerTy())
       return false;
     setDoesNotThrow(F);
     setDoesNotCapture(F, 1);
     setOnlyReadsMemory(F);
     break;
static bool inferPrototypeAttributes(Function &F, const TargetLibraryInfo &TLI) {
   case LibFunc::fprintf:
     if (FTy->getNumParams() < 2 || !FTy->getParamType(0)->isPointerTy() ||
         !FTy->getParamType(1)->isPointerTy())
       return false;
     setDoesNotThrow(F);
     setDoesNotCapture(F, 1);
     setDoesNotCapture(F, 2);
     setOnlyReadsMemory(F, 2);
+    setAlmostArgMemOnly(F);
     break;
   case LibFunc::fgetpos:
     if (FTy->getNumParams() < 2 || !FTy->getParamType(0)->isPointerTy() ||
         !FTy->getParamType(1)->isPointerTy())
       return false;
     setDoesNotThrow(F);
     setDoesNotCapture(F, 1);
     setDoesNotCapture(F, 2);
static bool inferPrototypeAttributes(Function &F, const TargetLibraryInfo &TLI) {
   case LibFunc::puts:
   case LibFunc::printf:
   case LibFunc::perror:
     if (FTy->getNumParams() != 1 || !FTy->getParamType(0)->isPointerTy())
       return false;
     setDoesNotThrow(F);
     setDoesNotCapture(F, 1);
     setOnlyReadsMemory(F, 1);
+    setAlmostArgMemOnly(F);
     break;
   case LibFunc::pread:
     if (FTy->getNumParams() != 4 || !FTy->getParamType(1)->isPointerTy())
       return false;
     // May throw; "pread" is a valid pthread cancellation point.
     setDoesNotCapture(F, 2);
     break;
   case LibFunc::pwrite:
if (FTy->getNumParams() != 2 || !FTy->getParamType(0)->isPointerTy() ||
       return false;
     // Currently some platforms have the restrict keyword on the arguments to
     // gettimeofday. To be conservative, do not add noalias to gettimeofday's
     // arguments.
     setDoesNotThrow(F);
     setDoesNotCapture(F, 1);
     setDoesNotCapture(F, 2);
     break;
+  case LibFunc::sin:
+  case LibFunc::cos:
+  case LibFunc::pow:
+  case LibFunc::sqrt:
+  case LibFunc::log:
+  case LibFunc::mulsc3:
+    setDoesNotThrow(F);
+    setDoesNotAccessMemory(F);
+    break;
+  case LibFunc::exit:
+    setDoesNotAccessMemory(F);
+    break;
   default:
     // Didn't mark any attributes.
     return false;
   }
 
   return true;
 }
 
for (auto *U : F->users())
       }
     return setDoesNotRecurse(*F);
   }
   return false;
 }
 
 static Attribute::AttrKind parseAttrKind(StringRef Kind) {
   return StringSwitch<Attribute::AttrKind>(Kind)
+    .Case("almostreadnone", Attribute::AlmostReadNone)
+    .Case("almostargmemonly", Attribute::AlmostArgMemOnly)
     .Case("alwaysinline", Attribute::AlwaysInline)
     .Case("builtin", Attribute::Builtin)
     .Case("cold", Attribute::Cold)
     .Case("convergent", Attribute::Convergent)
     .Case("inlinehint", Attribute::InlineHint)
     .Case("jumptable", Attribute::JumpTable)
     .Case("minsize", Attribute::MinSize)
     .Case("naked", Attribute::Naked)
bool FunctionAttrs::runOnSCC(CallGraphSCC &SCC) {
   Changed |= addArgumentAttrs(SCCNodes);
 
   // If we have no external nodes participating in the SCC, we can infer some
   // more precise attributes as well.
   if (!ExternalNode) {
     Changed |= addNoAliasAttrs(SCCNodes);
     Changed |= addNonNullAttrs(SCCNodes, *TLI);
   }
-  
   Changed |= addNoRecurseAttrs(SCC, Revisit);
   return Changed;
 }
 
 bool FunctionAttrs::doFinalization(CallGraph &CG) {
   bool Changed = false;
   // When iterating over SCCs we visit functions in a bottom-up fashion. Some of
   // the rules we have for identifying norecurse functions work best with a
   // top-down walk, so look again at all the functions we previously marked as
   // worth revisiting, in top-down order.
   for (auto &F : reverse(Revisit))
     if (F)
       Changed |= addNoRecurseAttrsTopDownOnly(cast<Function>((Value*)F));
   return Changed;
 }