[PATCH] Refactor AttributeSet::typeIncompatible to avoid creating new AttributeSets

[Hal Finkel]

Hi Pete,

attrbuilder-remove.diff:

+  /// \brief Remove the specified attributes at the specified index from this

+  /// attribute list. Since attribute lists are immutable, this returns the new

+  /// list.

Since -> Because (if you don't like starting a sentence with because, then reorder it)

+#ifndef NDEBUG

+  // 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!");

+#endif

No need to surround the assert with the ifdefs

Otherwise, LGTM.

type-incompatible.diff:

LGTM.

Thanks again,
Hal

----- Original Message -----
> From: "Pete Cooper" <peter_cooper at apple.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "llvm-commits" <llvm-commits at cs.uiuc.edu>
> Sent: Wednesday, April 22, 2015 8:12:14 PM
> Subject: [PATCH] Refactor AttributeSet::typeIncompatible to avoid creating new AttributeSets
> 
> Hi Hal
> 
> These 2 patches change the method typeIncompatible to avoid creating
> temporary AttributeSets.  This actually shows up as a fraction of a
> percent of compile time because the IR verify pass calls
> typeIncompatible to check the attributes on calls.
> 
> The first patch is attrbuilder-remove.diff which adds methods to the
> AttrBuilder itself to remove another AttrBuilder and to check if
> there is any overlap between 2 AttrBuilders.
> 
> The next patch - type-incompatible.diff - actually uses this new
> functionality to change that method to return an AttrBuilder and
> rewrite the few calls to it to work on an AttrBuilder instead of
> AttributeSet.
> 
> This is only the start of the work I’d like to do on AttributeSets.
>  There’s more to be done to reduce the number of sets we allocate
> (my current thinking involves adding an AttributeSetRef which
> references an index in to a set, and is safe because of the
> immutability of AttributeSet).  Let me know if you have anything
> else in mind.
> 
> Cheers,
> Pete
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory