Skip to content

Conversation

DavidSpickett
Copy link
Collaborator

@DavidSpickett DavidSpickett commented Oct 3, 2025

/home/david.spickett/llvm-project/llvm/lib/Analysis/HashRecognize.cpp:100:54: warning: comparison of integers of different signs:
'typename iterator_traits<ilist_iterator_w_bits<node_options<Instruction, true, false, void, true, BasicBlock>, false, false>>::difference_type' (aka 'int') and 'size_type' (aka 'unsigned int') [-Wsign-compare]
  100 |   return std::distance(Latch->begin(), Latch->end()) != Visited.size();
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~

By using Latch->size() instead.

/home/david.spickett/llvm-project/llvm/lib/Analysis/HashRecognize.cpp:100:54: warning:
comparison of integers of different signs:
'typename iterator_traits<ilist_iterator_w_bits<node_options<Instruction, true, false, void, true, BasicBlock>, false, false>>::difference_type' (aka 'int') and
'size_type' (aka 'unsigned int') [-Wsign-compare]
  100 |   return std::distance(Latch->begin(), Latch->end()) != Visited.size();
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~

With a static cast to size_t.

I thought about using Latch->size() but this skips a call to `It.setHeadBit(true);`
and I'm not sure whether that would make a difference in this context.
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Oct 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2025

@llvm/pr-subscribers-llvm-analysis

Author: David Spickett (DavidSpickett)

Changes
/home/david.spickett/llvm-project/llvm/lib/Analysis/HashRecognize.cpp:100:54: warning: comparison of integers of different signs:
'typename iterator_traits&lt;ilist_iterator_w_bits&lt;node_options&lt;Instruction, true, false, void, true, BasicBlock&gt;, false, false&gt;&gt;::difference_type' (aka 'int') and 'size_type' (aka 'unsigned int') [-Wsign-compare]
  100 |   return std::distance(Latch-&gt;begin(), Latch-&gt;end()) != Visited.size();
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~

With a static cast to size_t.

I thought about using Latch->size() but this skips a call to It.setHeadBit(true); and I'm not sure whether that would make a difference in this context.


Full diff: https://github.com/llvm/llvm-project/pull/161821.diff

1 Files Affected:

  • (modified) llvm/lib/Analysis/HashRecognize.cpp (+2-1)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 5d7ee1fe8eb12..613199a5d9f7b 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -97,7 +97,8 @@ static bool containsUnreachable(const Loop &L,
       }
     }
   }
-  return std::distance(Latch->begin(), Latch->end()) != Visited.size();
+  return static_cast<size_t>(std::distance(Latch->begin(), Latch->end())) !=
+         Visited.size();
 }
 
 /// A structure that can hold either a Simple Recurrence or a Conditional

Copy link
Contributor

@pfusik pfusik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about using Latch->size() but this skips a call to It.setHeadBit(true); and I'm not sure whether that would make a difference in this context.

Please use Latch->size().

@DavidSpickett
Copy link
Collaborator Author

Please use Latch->size().

Done.

Copy link
Contributor

@artagnon artagnon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Contributor

@pfusik pfusik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DavidSpickett DavidSpickett merged commit 78739ff into llvm:main Oct 3, 2025
9 checks passed
@DavidSpickett DavidSpickett deleted the clang-arm32 branch October 3, 2025 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants