Support align attribute for return values

[apilipenko]

Summary

Since D9791 we take alignment into account to decide if it's safe to speculate load. Now pointer has to be both dereferenceable and properly aligned to be speculative loadable. It means that we want to support align attribute everywhere we support dereferenceable attribute.

Align attribute was supported for function arguments only. Support this attribute for return values as well.

[reames]

lib/AsmParser/LLParser.cpp

1382 ↗

(On Diff #34684)

The fact your changing the IR parser and not the IR writer, bitcode parser, or bitcode writer seems mildly suspect. Have you tested the ability to round trip through both IR and Bitcode?

test/Analysis/ValueTracking/memory-dereferenceable.ll

120 ↗

(On Diff #34684)

The syntax here feels really weird for me. "align 16" vs "align(16)" feels very inconsistent with "dereferenceable(32)". Not saying we need to fix that right now, just noting it.

[apilipenko]

lib/AsmParser/LLParser.cpp

1382 ↗

(On Diff #34684)

Yes, I have. Other parts work fine.

[apilipenko]

test/Analysis/ValueTracking/memory-dereferenceable.ll

120 ↗

(On Diff #34684)

BTW, do we care about backward compatibility of textual representation?

[reames]

LGTM

test/Analysis/ValueTracking/memory-dereferenceable.ll

120 ↗

(On Diff #34684)

Not really. I'd say don't break it needlessly, but the IR is a living format with no compatibility guarantees. Provided all the in-tree tests have been updated, that's it. (If you are going to change format, please do so in a separate change.)

llvm/trunk/lib/Analysis/ValueTracking.cpp

static bool isAligned(const Value *Base, APInt Offset, unsigned Align,
                       const DataLayout &DL) {
   APInt BaseAlign(Offset.getBitWidth(), 0);
   if (const AllocaInst *AI = dyn_cast<AllocaInst>(Base))
     BaseAlign = AI->getAlignment();
   else if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(Base))
     BaseAlign = GV->getAlignment();
   else if (const Argument *A = dyn_cast<Argument>(Base))
     BaseAlign = A->getParamAlignment();
+  else if (auto CS = ImmutableCallSite(Base))
+    BaseAlign = CS.getAttributes().getParamAlignment(AttributeSet::ReturnIndex);
 
   if (!BaseAlign) {
     Type *Ty = Base->getType()->getPointerElementType();
     BaseAlign = DL.getABITypeAlignment(Ty);
   }
 
   APInt Alignment(Offset.getBitWidth(), Align);
 

llvm/trunk/lib/AsmParser/LLParser.cpp

while (1) {
     }
     case lltok::kw_dereferenceable_or_null: {
       uint64_t Bytes;
       if (ParseOptionalDerefAttrBytes(lltok::kw_dereferenceable_or_null, Bytes))
         return true;
       B.addDereferenceableOrNullAttr(Bytes);
       continue;
     }
+    case lltok::kw_align: {
+      unsigned Alignment;
+      if (ParseOptionalAlignment(Alignment))
+        return true;
+      B.addAlignmentAttr(Alignment);
+      continue;
+    }
     case lltok::kw_inreg:           B.addAttribute(Attribute::InReg); break;
     case lltok::kw_noalias:         B.addAttribute(Attribute::NoAlias); break;
     case lltok::kw_nonnull:         B.addAttribute(Attribute::NonNull); break;
     case lltok::kw_signext:         B.addAttribute(Attribute::SExt); break;
     case lltok::kw_zeroext:         B.addAttribute(Attribute::ZExt); break;
 
     // Error handling.
-    case lltok::kw_align:
     case lltok::kw_byval:
     case lltok::kw_inalloca:
     case lltok::kw_nest:
     case lltok::kw_nocapture:
     case lltok::kw_returned:
     case lltok::kw_sret:
       HaveError |= Error(Lex.getLoc(), "invalid use of parameter-only attribute");
       break;

llvm/trunk/test/Analysis/ValueTracking/memory-dereferenceable.ll

 ; RUN: opt -print-memderefs -analyze -S <%s | FileCheck %s
 
 ; Uses the print-deref (+ analyze to print) pass to run
 ; isDereferenceablePointer() on many load instruction operands
 
 target datalayout = "e"
 
 declare zeroext i1 @return_i1()
 
+declare i32* @foo()
 @globalstr = global [6 x i8] c"hello\00"
 @globali32ptr = external global i32*
 
 %struct.A = type { [8 x i8], [5 x i8] }
 @globalstruct = external global %struct.A
 
 @globalptr.align1 = external global i8, align 1
 @globalptr.align16 = external global i8, align 16
; CHECK: %gep.align16.offset16{{.*}}(aligned)
     %gep.align16.offset1 = getelementptr inbounds i8, i8 addrspace(1)* %dparam.align16, i32 1
     %gep.align1.offset16 = getelementptr inbounds i8, i8 addrspace(1)* %dparam.align1, i32 16
     %gep.align16.offset16 = getelementptr inbounds i8, i8 addrspace(1)* %dparam.align16, i32 16
     %load19 = load i8, i8 addrspace(1)* %gep.align1.offset1, align 16
     %load20 = load i8, i8 addrspace(1)* %gep.align16.offset1, align 16
     %load21 = load i8, i8 addrspace(1)* %gep.align1.offset16, align 16
     %load22 = load i8, i8 addrspace(1)* %gep.align16.offset16, align 16
 
+; CHECK-NOT: %no_deref_return
+; CHECK: %deref_return{{.*}}(unaligned)
+; CHECK: %deref_and_aligned_return{{.*}}(aligned)
+    %no_deref_return = call i32* @foo()
+    %deref_return = call dereferenceable(32) i32* @foo()
+    %deref_and_aligned_return = call dereferenceable(32) align 16 i32* @foo()
+    %load23 = load i32, i32* %no_deref_return
+    %load24 = load i32, i32* %deref_return, align 16
+    %load25 = load i32, i32* %deref_and_aligned_return, align 16
+
     ret void
 }
 
 declare i32 @llvm.experimental.gc.statepoint.p0f_i1f(i64, i32, i1 ()*, i32, i32, ...)
 declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32)
 
 !0 = !{i64 4}
 !1 = !{i64 2}
 !2 = !{}