Skip to content

[NFCI][LLVM] Adopt ArrayRef::consume_front() in a few places #146793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 4, 2025

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Jul 2, 2025

No description provided.

@jurahul jurahul force-pushed the adopt_consume_front branch 2 times, most recently from aef5d39 to 2443ae6 Compare July 2, 2025 23:14
@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2025

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-debuginfo

Author: Rahul Joshi (jurahul)

Changes

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

5 Files Affected:

  • (modified) llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h (+4-8)
  • (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+7-12)
  • (modified) llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/MachinePostDominators.cpp (+2-2)
  • (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+6-6)
diff --git a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
index f5f6fe69430cc..c2897891544af 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
@@ -240,8 +240,7 @@ struct BinaryAnnotationIterator
     if (Annotations.empty())
       return -1;
 
-    uint8_t FirstByte = Annotations.front();
-    Annotations = Annotations.drop_front();
+    uint8_t FirstByte = Annotations.consume_front();
 
     if ((FirstByte & 0x80) == 0x00)
       return FirstByte;
@@ -249,8 +248,7 @@ struct BinaryAnnotationIterator
     if (Annotations.empty())
       return -1;
 
-    uint8_t SecondByte = Annotations.front();
-    Annotations = Annotations.drop_front();
+    uint8_t SecondByte = Annotations.consume_front();
 
     if ((FirstByte & 0xC0) == 0x80)
       return ((FirstByte & 0x3F) << 8) | SecondByte;
@@ -258,14 +256,12 @@ struct BinaryAnnotationIterator
     if (Annotations.empty())
       return -1;
 
-    uint8_t ThirdByte = Annotations.front();
-    Annotations = Annotations.drop_front();
+    uint8_t ThirdByte = Annotations.consume_front();
 
     if (Annotations.empty())
       return -1;
 
-    uint8_t FourthByte = Annotations.front();
-    Annotations = Annotations.drop_front();
+    uint8_t FourthByte = Annotations.consume_front();
 
     if ((FirstByte & 0xE0) == 0xC0)
       return ((FirstByte & 0x1F) << 24) | (SecondByte << 16) |
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index de7bf9b7f4d79..13cec5424a372 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7513,11 +7513,9 @@ std::vector<FunctionSummary::ParamAccess>
 ModuleSummaryIndexBitcodeReader::parseParamAccesses(ArrayRef<uint64_t> Record) {
   auto ReadRange = [&]() {
     APInt Lower(FunctionSummary::ParamAccess::RangeWidth,
-                BitcodeReader::decodeSignRotatedValue(Record.front()));
-    Record = Record.drop_front();
+                BitcodeReader::decodeSignRotatedValue(Record.consume_front()));
     APInt Upper(FunctionSummary::ParamAccess::RangeWidth,
-                BitcodeReader::decodeSignRotatedValue(Record.front()));
-    Record = Record.drop_front();
+                BitcodeReader::decodeSignRotatedValue(Record.consume_front()));
     ConstantRange Range{Lower, Upper};
     assert(!Range.isFullSet());
     assert(!Range.isUpperSignWrapped());
@@ -7528,16 +7526,13 @@ ModuleSummaryIndexBitcodeReader::parseParamAccesses(ArrayRef<uint64_t> Record) {
   while (!Record.empty()) {
     PendingParamAccesses.emplace_back();
     FunctionSummary::ParamAccess &ParamAccess = PendingParamAccesses.back();
-    ParamAccess.ParamNo = Record.front();
-    Record = Record.drop_front();
+    ParamAccess.ParamNo = Record.consume_front();
     ParamAccess.Use = ReadRange();
-    ParamAccess.Calls.resize(Record.front());
-    Record = Record.drop_front();
+    ParamAccess.Calls.resize(Record.consume_front());
     for (auto &Call : ParamAccess.Calls) {
-      Call.ParamNo = Record.front();
-      Record = Record.drop_front();
-      Call.Callee = std::get<0>(getValueInfoFromValueId(Record.front()));
-      Record = Record.drop_front();
+      Call.ParamNo = Record.consume_front();
+      Call.Callee =
+          std::get<0>(getValueInfoFromValueId(Record.consume_front()));
       Call.Offsets = ReadRange();
     }
   }
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index 5e1b313b4d2fa..d37a22f321cef 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -2085,8 +2085,8 @@ TypeIndex CodeViewDebug::lowerTypeFunction(const DISubroutineType *Ty) {
   ArrayRef<TypeIndex> ArgTypeIndices = {};
   if (!ReturnAndArgTypeIndices.empty()) {
     auto ReturnAndArgTypesRef = ArrayRef(ReturnAndArgTypeIndices);
-    ReturnTypeIndex = ReturnAndArgTypesRef.front();
-    ArgTypeIndices = ReturnAndArgTypesRef.drop_front();
+    ReturnTypeIndex = ReturnAndArgTypesRef.consume_front();
+    ArgTypeIndices = ReturnAndArgTypesRef;
   }
 
   ArgListRecord ArgListRec(TypeRecordKind::ArgList, ArgTypeIndices);
diff --git a/llvm/lib/CodeGen/MachinePostDominators.cpp b/llvm/lib/CodeGen/MachinePostDominators.cpp
index 1cb7e465881a2..d6b5ee0e46d74 100644
--- a/llvm/lib/CodeGen/MachinePostDominators.cpp
+++ b/llvm/lib/CodeGen/MachinePostDominators.cpp
@@ -99,8 +99,8 @@ MachineBasicBlock *MachinePostDominatorTree::findNearestCommonDominator(
     ArrayRef<MachineBasicBlock *> Blocks) const {
   assert(!Blocks.empty());
 
-  MachineBasicBlock *NCD = Blocks.front();
-  for (MachineBasicBlock *BB : Blocks.drop_front()) {
+  MachineBasicBlock *NCD = Blocks.consume_front();
+  for (MachineBasicBlock *BB : Blocks) {
     NCD = Base::findNearestCommonDominator(NCD, BB);
 
     // Stop when the root is reached.
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 0941bf61953f1..bb09083bbc73b 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -1511,8 +1511,8 @@ static InstructionsState getSameOpcode(ArrayRef<Value *> VL,
 /// \returns true if all of the values in \p VL have the same type or false
 /// otherwise.
 static bool allSameType(ArrayRef<Value *> VL) {
-  Type *Ty = VL.front()->getType();
-  return all_of(VL.drop_front(), [&](Value *V) { return V->getType() == Ty; });
+  Type *Ty = VL.consume_front()->getType();
+  return all_of(VL, [&](Value *V) { return V->getType() == Ty; });
 }
 
 /// \returns True if in-tree use also needs extract. This refers to
@@ -5567,8 +5567,8 @@ static bool arePointersCompatible(Value *Ptr1, Value *Ptr2,
 /// Calculates minimal alignment as a common alignment.
 template <typename T>
 static Align computeCommonAlignment(ArrayRef<Value *> VL) {
-  Align CommonAlignment = cast<T>(VL.front())->getAlign();
-  for (Value *V : VL.drop_front())
+  Align CommonAlignment = cast<T>(VL.consume_front())->getAlign();
+  for (Value *V : VL)
     CommonAlignment = std::min(CommonAlignment, cast<T>(V)->getAlign());
   return CommonAlignment;
 }
@@ -9483,8 +9483,8 @@ class PHIHandler {
       ArrayRef<unsigned> IncomingValues = P.second;
       if (IncomingValues.size() <= 1)
         continue;
-      unsigned BasicI = IncomingValues.front();
-      for (unsigned I : IncomingValues.drop_front()) {
+      unsigned BasicI = IncomingValues.consume_front();
+      for (unsigned I : IncomingValues) {
         assert(all_of(enumerate(Operands[I]),
                       [&](const auto &Data) {
                         return !Data.value() ||

@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2025

@llvm/pr-subscribers-vectorizers

Author: Rahul Joshi (jurahul)

Changes

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

5 Files Affected:

  • (modified) llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h (+4-8)
  • (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+7-12)
  • (modified) llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/MachinePostDominators.cpp (+2-2)
  • (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+6-6)
diff --git a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
index f5f6fe69430cc..c2897891544af 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
@@ -240,8 +240,7 @@ struct BinaryAnnotationIterator
     if (Annotations.empty())
       return -1;
 
-    uint8_t FirstByte = Annotations.front();
-    Annotations = Annotations.drop_front();
+    uint8_t FirstByte = Annotations.consume_front();
 
     if ((FirstByte & 0x80) == 0x00)
       return FirstByte;
@@ -249,8 +248,7 @@ struct BinaryAnnotationIterator
     if (Annotations.empty())
       return -1;
 
-    uint8_t SecondByte = Annotations.front();
-    Annotations = Annotations.drop_front();
+    uint8_t SecondByte = Annotations.consume_front();
 
     if ((FirstByte & 0xC0) == 0x80)
       return ((FirstByte & 0x3F) << 8) | SecondByte;
@@ -258,14 +256,12 @@ struct BinaryAnnotationIterator
     if (Annotations.empty())
       return -1;
 
-    uint8_t ThirdByte = Annotations.front();
-    Annotations = Annotations.drop_front();
+    uint8_t ThirdByte = Annotations.consume_front();
 
     if (Annotations.empty())
       return -1;
 
-    uint8_t FourthByte = Annotations.front();
-    Annotations = Annotations.drop_front();
+    uint8_t FourthByte = Annotations.consume_front();
 
     if ((FirstByte & 0xE0) == 0xC0)
       return ((FirstByte & 0x1F) << 24) | (SecondByte << 16) |
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index de7bf9b7f4d79..13cec5424a372 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -7513,11 +7513,9 @@ std::vector<FunctionSummary::ParamAccess>
 ModuleSummaryIndexBitcodeReader::parseParamAccesses(ArrayRef<uint64_t> Record) {
   auto ReadRange = [&]() {
     APInt Lower(FunctionSummary::ParamAccess::RangeWidth,
-                BitcodeReader::decodeSignRotatedValue(Record.front()));
-    Record = Record.drop_front();
+                BitcodeReader::decodeSignRotatedValue(Record.consume_front()));
     APInt Upper(FunctionSummary::ParamAccess::RangeWidth,
-                BitcodeReader::decodeSignRotatedValue(Record.front()));
-    Record = Record.drop_front();
+                BitcodeReader::decodeSignRotatedValue(Record.consume_front()));
     ConstantRange Range{Lower, Upper};
     assert(!Range.isFullSet());
     assert(!Range.isUpperSignWrapped());
@@ -7528,16 +7526,13 @@ ModuleSummaryIndexBitcodeReader::parseParamAccesses(ArrayRef<uint64_t> Record) {
   while (!Record.empty()) {
     PendingParamAccesses.emplace_back();
     FunctionSummary::ParamAccess &ParamAccess = PendingParamAccesses.back();
-    ParamAccess.ParamNo = Record.front();
-    Record = Record.drop_front();
+    ParamAccess.ParamNo = Record.consume_front();
     ParamAccess.Use = ReadRange();
-    ParamAccess.Calls.resize(Record.front());
-    Record = Record.drop_front();
+    ParamAccess.Calls.resize(Record.consume_front());
     for (auto &Call : ParamAccess.Calls) {
-      Call.ParamNo = Record.front();
-      Record = Record.drop_front();
-      Call.Callee = std::get<0>(getValueInfoFromValueId(Record.front()));
-      Record = Record.drop_front();
+      Call.ParamNo = Record.consume_front();
+      Call.Callee =
+          std::get<0>(getValueInfoFromValueId(Record.consume_front()));
       Call.Offsets = ReadRange();
     }
   }
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index 5e1b313b4d2fa..d37a22f321cef 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -2085,8 +2085,8 @@ TypeIndex CodeViewDebug::lowerTypeFunction(const DISubroutineType *Ty) {
   ArrayRef<TypeIndex> ArgTypeIndices = {};
   if (!ReturnAndArgTypeIndices.empty()) {
     auto ReturnAndArgTypesRef = ArrayRef(ReturnAndArgTypeIndices);
-    ReturnTypeIndex = ReturnAndArgTypesRef.front();
-    ArgTypeIndices = ReturnAndArgTypesRef.drop_front();
+    ReturnTypeIndex = ReturnAndArgTypesRef.consume_front();
+    ArgTypeIndices = ReturnAndArgTypesRef;
   }
 
   ArgListRecord ArgListRec(TypeRecordKind::ArgList, ArgTypeIndices);
diff --git a/llvm/lib/CodeGen/MachinePostDominators.cpp b/llvm/lib/CodeGen/MachinePostDominators.cpp
index 1cb7e465881a2..d6b5ee0e46d74 100644
--- a/llvm/lib/CodeGen/MachinePostDominators.cpp
+++ b/llvm/lib/CodeGen/MachinePostDominators.cpp
@@ -99,8 +99,8 @@ MachineBasicBlock *MachinePostDominatorTree::findNearestCommonDominator(
     ArrayRef<MachineBasicBlock *> Blocks) const {
   assert(!Blocks.empty());
 
-  MachineBasicBlock *NCD = Blocks.front();
-  for (MachineBasicBlock *BB : Blocks.drop_front()) {
+  MachineBasicBlock *NCD = Blocks.consume_front();
+  for (MachineBasicBlock *BB : Blocks) {
     NCD = Base::findNearestCommonDominator(NCD, BB);
 
     // Stop when the root is reached.
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 0941bf61953f1..bb09083bbc73b 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -1511,8 +1511,8 @@ static InstructionsState getSameOpcode(ArrayRef<Value *> VL,
 /// \returns true if all of the values in \p VL have the same type or false
 /// otherwise.
 static bool allSameType(ArrayRef<Value *> VL) {
-  Type *Ty = VL.front()->getType();
-  return all_of(VL.drop_front(), [&](Value *V) { return V->getType() == Ty; });
+  Type *Ty = VL.consume_front()->getType();
+  return all_of(VL, [&](Value *V) { return V->getType() == Ty; });
 }
 
 /// \returns True if in-tree use also needs extract. This refers to
@@ -5567,8 +5567,8 @@ static bool arePointersCompatible(Value *Ptr1, Value *Ptr2,
 /// Calculates minimal alignment as a common alignment.
 template <typename T>
 static Align computeCommonAlignment(ArrayRef<Value *> VL) {
-  Align CommonAlignment = cast<T>(VL.front())->getAlign();
-  for (Value *V : VL.drop_front())
+  Align CommonAlignment = cast<T>(VL.consume_front())->getAlign();
+  for (Value *V : VL)
     CommonAlignment = std::min(CommonAlignment, cast<T>(V)->getAlign());
   return CommonAlignment;
 }
@@ -9483,8 +9483,8 @@ class PHIHandler {
       ArrayRef<unsigned> IncomingValues = P.second;
       if (IncomingValues.size() <= 1)
         continue;
-      unsigned BasicI = IncomingValues.front();
-      for (unsigned I : IncomingValues.drop_front()) {
+      unsigned BasicI = IncomingValues.consume_front();
+      for (unsigned I : IncomingValues) {
         assert(all_of(enumerate(Operands[I]),
                       [&](const auto &Data) {
                         return !Data.value() ||

@kazutakahirata
Copy link
Contributor

@jurahul @kuhar Is it too late to suggest renaming consume_front and consume_back to pop_front and pop_back, respectively? The pop_front and pop_back are "household names" as we have std::list::{pop_front,pop_back} and std::vector::pop_back. Aside from a few consume* in StringRef.h, we don't have any consume* under llvm/include/llvm/ADT.

@jurahul
Copy link
Contributor Author

jurahul commented Jul 4, 2025

@jurahul @kuhar Is it too late to suggest renaming consume_front and consume_back to pop_front and pop_back, respectively? The pop_front and pop_back are "household names" as we have std::list::{pop_front,pop_back} and std::vector::pop_back. Aside from a few consume* in StringRef.h, we don't have any consume* under llvm/include/llvm/ADT.

Its not :) It was just added yesterday. Though the std::pop_front() and pop_back() do not return the element being popped. The equivalent in SmallVector seems to be pop_back_val(). So we could rename them to be pop_front_val() and pop_back_val().

@kazutakahirata
Copy link
Contributor

@jurahul @kuhar Is it too late to suggest renaming consume_front and consume_back to pop_front and pop_back, respectively? The pop_front and pop_back are "household names" as we have std::list::{pop_front,pop_back} and std::vector::pop_back. Aside from a few consume* in StringRef.h, we don't have any consume* under llvm/include/llvm/ADT.

Its not :) It was just added yesterday. Though the std::pop_front() and pop_back() do not return the element being popped. The equivalent in SmallVector seems to be pop_back_val(). So we could rename them to be pop_front_val() and pop_back_val().

OK. pop_front_val and pop_back_val sound good to me. Let's wait to hear from @kuhar.

@jurahul
Copy link
Contributor Author

jurahul commented Jul 4, 2025

@kuhar please see the thread above, we want to rename these to pop_back_val and pop_front_val. WDYT?

@kuhar
Copy link
Member

kuhar commented Jul 4, 2025

I'm -1 on the renaming -- pop_*_val change the underlying container contents, whereas consume_ only adjust pointers. This is consistent with the naming in StringRef.

@jurahul
Copy link
Contributor Author

jurahul commented Jul 4, 2025

Abstractly, the contents of the ArrayRef do change after this. It has one less element. The fact that it's achieved by pointer manipulation is a detail because ArrayRef does not own the underlying data.

However, I am ok either way in the renaming.

@kuhar
Copy link
Member

kuhar commented Jul 4, 2025

Abstractly, the contents of the ArrayRef do change after this. It has one less element. The fact that it's achieved by pointer manipulation is a detail because ArrayRef does not own the underlying data.

I don't think we should think about modifying the contents at all since it's a view type of container. And ArrayRef specifically is immutable wrt the contents. Similarly, I wouldn't want to rename drop_front to pop_front. These things come in pairs: pop_front() and pop_front_val(), and drop_front() and consume_front().

@jurahul
Copy link
Contributor Author

jurahul commented Jul 4, 2025

Sounds good. Will wait to @kazutakahirata to ack before merging

@kazutakahirata
Copy link
Contributor

Sounds good. Will wait to @kazutakahirata to ack before merging

@jurahul OK. Please proceed with this PR. Thanks!

@jurahul jurahul merged commit b38de6c into llvm:main Jul 4, 2025
15 checks passed
@dwblaikie
Copy link
Collaborator

I think the view does change, so pop could make sense - but mostly I'd advocate for: do whatever std::span does... but doesn't look like span has any of these sort of operations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants