[IR] Short-circuit comparison with itself for Attributes

[danilaml]

llvm/lib/IR/Attributes.cpp

614

This avoids potential assert here (which can be triggered when glibc tests irreflexivity with expensive checks enabled).

[dblaikie]

Can this be covered in LLVM's regression tests?

[danilaml]

@dblaikie It was actually failing on some of the lit tests when compiler was compiled with EXPENSIVE_CHECKS enabled (kind of how I discovered it), but after this recent refactor which removed a sort step: https://github.com/llvm/llvm-project/commit/ed766f1bb1040a520fb5646ab75851e2b0fd66df#diff-7b43e801c5abbb03d571f9aad6af63a5L792
I don't know if the codepath with ::sort (which has those checks) can be hit from llvm IR. Instead, I've added an explicit check to AttributesTest.cpp that currently asserts.

[danilaml]

ping

[jdoerfert]

LGTM

llvm/lib/IR/Attributes.cpp

 }
 
 Type *AttributeImpl::getValueAsType() const {
   assert(isTypeAttribute());
   return static_cast<const TypeAttributeImpl *>(this)->getTypeValue();
 }
 
 bool AttributeImpl::operator<(const AttributeImpl &AI) const {
+  if (this == &AI)
+    return false;
   // This sorts the attributes with Attribute::AttrKinds coming first (sorted
   // relative to their enum value) and then strings.
   if (isEnumAttribute()) {
     if (AI.isEnumAttribute()) return getKindAsEnum() < AI.getKindAsEnum();
     if (AI.isIntAttribute()) return true;
     if (AI.isStringAttribute()) return true;
     if (AI.isTypeAttribute()) return true;
   }
 
   if (isTypeAttribute()) {
     if (AI.isEnumAttribute()) return false;
     if (AI.isTypeAttribute()) {
       assert(getKindAsEnum() != AI.getKindAsEnum() &&
              "Comparison of types would be unstable");
       return getKindAsEnum() < AI.getKindAsEnum();
     }
     if (AI.isIntAttribute()) return true;
     if (AI.isStringAttribute()) return true;
   }
 
   if (isIntAttribute()) {

llvm/unittests/IR/AttributesTest.cpp

TEST(Attributes, Ordering) {
   EXPECT_TRUE(Align4 < Align5);
   EXPECT_TRUE(Align4 < Deref4);
   EXPECT_TRUE(Align4 < Deref5);
   EXPECT_TRUE(Align5 < Deref4);
 
   Attribute ByVal = Attribute::get(C, Attribute::ByVal, Type::getInt32Ty(C));
   EXPECT_FALSE(ByVal < Attribute::get(C, Attribute::ZExt));
   EXPECT_TRUE(ByVal < Align4);
+  EXPECT_FALSE(ByVal < ByVal);
 
   AttributeList ASs[] = {AttributeList::get(C, 2, Attribute::ZExt),
                          AttributeList::get(C, 1, Attribute::SExt)};
 
   AttributeList SetA = AttributeList::get(C, ASs);
   AttributeList SetB = SetA.removeAttributes(C, 1, ASs[1].getAttributes(1));
   EXPECT_NE(SetA, SetB);
 }