[llvm] r236666 - Add remove method to operate on AttrBuilder instead of AttributeSet.

[Pete Cooper]

Thanks for the fix! I was still trying to understand what the bot was reporting.

Pete

Sent from my iPhone

> On May 6, 2015, at 6:02 PM, Justin Bogner <mail at justinbogner.com> wrote:
> 
> Pete Cooper <peter_cooper at apple.com> writes:
>> Author: pete
>> Date: Wed May  6 18:19:43 2015
>> New Revision: 236666
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=236666&view=rev
>> Log:
>> Add remove method to operate on AttrBuilder instead of AttributeSet.
>> 
>> Prior to this change we would have to construct a temporary
>> AttributeSet (which isn't temporary at all given that its allocated on
>> the context), just to contain the attributes in the builder, then call
>> remove on that.
>> 
>> Now we can just remove any attributes from the (lightweight and really
>> temporary) builder itself.
>> 
>> Will be used in a future commit to remove some temporary attributes sets.
> 
> An MSAN bot was sad after this change:
> 
>    http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/3678
> 
> Looks like the AttrBuilder constructors forgot to initialize a field, so
> I made an attempt to fix it in r236686.
> 
>> Modified:
>>    llvm/trunk/include/llvm/IR/Attributes.h
>>    llvm/trunk/lib/IR/Attributes.cpp
>> 
>> Modified: llvm/trunk/include/llvm/IR/Attributes.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Attributes.h?rev=236666&r1=236665&r2=236666&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/Attributes.h (original)
>> +++ llvm/trunk/include/llvm/IR/Attributes.h Wed May  6 18:19:43 2015
>> @@ -289,6 +289,12 @@ public:
>>   AttributeSet removeAttributes(LLVMContext &C, unsigned Index, 
>>                                 AttributeSet Attrs) const;
>> 
>> +  /// \brief Remove the specified attributes at the specified index from this
>> +  /// attribute list. Because attribute lists are immutable, this returns the
>> +  /// new list.
>> +  AttributeSet removeAttributes(LLVMContext &C, unsigned Index,
>> +                                const AttrBuilder &Attrs) const;
>> +
>>   /// \brief Add the dereferenceable attribute to the attribute set at the given
>>   /// index. Since attribute sets are immutable, this returns a new set.
>>   AttributeSet addDereferenceableAttr(LLVMContext &C, unsigned Index,
>> @@ -464,6 +470,13 @@ public:
>>   /// \brief Add the attributes from the builder.
>>   AttrBuilder &merge(const AttrBuilder &B);
>> 
>> +  /// \brief Remove the attributes from the builder.
>> +  AttrBuilder &remove(const AttrBuilder &B);
>> +
>> +  /// \brief \brief Return true if the builder has any attribute that's in the
>> +  /// specified builder.
>> +  bool overlaps(const AttrBuilder &B) const;
>> +
>>   /// \brief Return true if the builder has the specified attribute.
>>   bool contains(Attribute::AttrKind A) const {
>>     assert((unsigned)A < Attribute::EndAttrKinds && "Attribute out of range!");
>> 
>> Modified: llvm/trunk/lib/IR/Attributes.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Attributes.cpp?rev=236666&r1=236665&r2=236666&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/Attributes.cpp (original)
>> +++ llvm/trunk/lib/IR/Attributes.cpp Wed May  6 18:19:43 2015
>> @@ -857,6 +857,42 @@ AttributeSet AttributeSet::removeAttribu
>>   return get(C, AttrSet);
>> }
>> 
>> +AttributeSet AttributeSet::removeAttributes(LLVMContext &C, unsigned Index,
>> +                                            const AttrBuilder &Attrs) const {
>> +  if (!pImpl) return AttributeSet();
>> +
>> +  // FIXME it is not obvious how this should work for alignment.
>> +  // For now, say we can't pass in alignment, which no current use does.
>> +  assert(!Attrs.hasAlignmentAttr() && "Attempt to change alignment!");
>> +
>> +  // Add the attribute slots before the one we're trying to add.
>> +  SmallVector<AttributeSet, 4> AttrSet;
>> +  uint64_t NumAttrs = pImpl->getNumAttributes();
>> +  AttributeSet AS;
>> +  uint64_t LastIndex = 0;
>> +  for (unsigned I = 0, E = NumAttrs; I != E; ++I) {
>> +    if (getSlotIndex(I) >= Index) {
>> +      if (getSlotIndex(I) == Index) AS = getSlotAttributes(LastIndex++);
>> +      break;
>> +    }
>> +    LastIndex = I + 1;
>> +    AttrSet.push_back(getSlotAttributes(I));
>> +  }
>> +
>> +  // Now remove the attribute from the correct slot. There may already be an
>> +  // AttributeSet there.
>> +  AttrBuilder B(AS, Index);
>> +  B.remove(Attrs);
>> +
>> +  AttrSet.push_back(AttributeSet::get(C, Index, B));
>> +
>> +  // Add the remaining attribute slots.
>> +  for (unsigned I = LastIndex, E = NumAttrs; I < E; ++I)
>> +    AttrSet.push_back(getSlotAttributes(I));
>> +
>> +  return get(C, AttrSet);
>> +}
>> +
>> AttributeSet AttributeSet::addDereferenceableAttr(LLVMContext &C, unsigned Index,
>>                                                   uint64_t Bytes) const {
>>   llvm::AttrBuilder B;
>> @@ -1211,15 +1247,52 @@ AttrBuilder &AttrBuilder::merge(const At
>>   if (!DerefBytes)
>>     DerefBytes = B.DerefBytes;
>> 
>> +  if (!DerefOrNullBytes)
>> +    DerefOrNullBytes = B.DerefOrNullBytes;
>> +
>>   Attrs |= B.Attrs;
>> 
>> -  for (td_const_iterator I = B.TargetDepAttrs.begin(),
>> -         E = B.TargetDepAttrs.end(); I != E; ++I)
>> -    TargetDepAttrs[I->first] = I->second;
>> +  for (auto I : B.td_attrs())
>> +    TargetDepAttrs[I.first] = I.second;
>> +
>> +  return *this;
>> +}
>> +
>> +AttrBuilder &AttrBuilder::remove(const AttrBuilder &B) {
>> +  // FIXME: What if both have alignments, but they don't match?!
>> +  if (B.Alignment)
>> +    Alignment = 0;
>> +
>> +  if (B.StackAlignment)
>> +    StackAlignment = 0;
>> +
>> +  if (B.DerefBytes)
>> +    DerefBytes = 0;
>> +
>> +  if (B.DerefOrNullBytes)
>> +    DerefOrNullBytes = 0;
>> +
>> +  Attrs &= ~B.Attrs;
>> +
>> +  for (auto I : B.td_attrs())
>> +    TargetDepAttrs.erase(I.first);
>> 
>>   return *this;
>> }
>> 
>> +bool AttrBuilder::overlaps(const AttrBuilder &B) const {
>> +  // First check if any of the target independent attributes overlap.
>> +  if ((Attrs & B.Attrs).any())
>> +    return true;
>> +
>> +  // Then check if any target dependent ones do.
>> +  for (auto I : td_attrs())
>> +    if (B.contains(I.first))
>> +      return true;
>> +
>> +  return false;
>> +}
>> +
>> bool AttrBuilder::contains(StringRef A) const {
>>   return TargetDepAttrs.find(A) != TargetDepAttrs.end();
>> }
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits