[MC] Optimize loops in MC. #98114

chestnykh · 2024-07-09T05:51:29Z

https://llvm.org/docs/CodingStandards.html tells us that we should avoid evaluating .end() each time if possible.

llvmbot · 2024-07-09T05:51:58Z

@llvm/pr-subscribers-mc

Author: Dmitriy Chestnykh (chestnykh)

Changes

https://llvm.org/docs/CodingStandards.html tells us that we should avoid evaluating .end() each time if possible.

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

3 Files Affected:

(modified) llvm/lib/MC/MCParser/AsmParser.cpp (+3-3)
(modified) llvm/lib/MC/MCParser/MasmParser.cpp (+3-3)
(modified) llvm/lib/MC/XCOFFObjectWriter.cpp (+14-11)

diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index f3caa90eedfb1..dba56b15424d5 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -6116,8 +6116,8 @@ bool AsmParser::parseMSInlineAsm(
   const char *AsmStart = ASMString.begin();
   const char *AsmEnd = ASMString.end();
   array_pod_sort(AsmStrRewrites.begin(), AsmStrRewrites.end(), rewritesSort);
-  for (auto it = AsmStrRewrites.begin(); it != AsmStrRewrites.end(); ++it) {
-    const AsmRewrite &AR = *it;
+  for (auto I = AsmStrRewrites.begin(), E = AsmStrRewrites.end(); I != E; ++I) {
+    const AsmRewrite &AR = *I;
     // Check if this has already been covered by another rewrite...
     if (AR.Done)
       continue;
@@ -6160,7 +6160,7 @@ bool AsmParser::parseMSInlineAsm(
         SMLoc OffsetLoc = SMLoc::getFromPointer(AR.IntelExp.OffsetName.data());
         size_t OffsetLen = OffsetName.size();
         auto rewrite_it = std::find_if(
-            it, AsmStrRewrites.end(), [&](const AsmRewrite &FusingAR) {
+            I, AsmStrRewrites.end(), [&](const AsmRewrite &FusingAR) {
               return FusingAR.Loc == OffsetLoc && FusingAR.Len == OffsetLen &&
                      (FusingAR.Kind == AOK_Input ||
                       FusingAR.Kind == AOK_CallInput);
diff --git a/llvm/lib/MC/MCParser/MasmParser.cpp b/llvm/lib/MC/MCParser/MasmParser.cpp
index 653cc64b4c36a..f64b7f62d61d0 100644
--- a/llvm/lib/MC/MCParser/MasmParser.cpp
+++ b/llvm/lib/MC/MCParser/MasmParser.cpp
@@ -7474,8 +7474,8 @@ bool MasmParser::parseMSInlineAsm(
   const char *AsmStart = ASMString.begin();
   const char *AsmEnd = ASMString.end();
   array_pod_sort(AsmStrRewrites.begin(), AsmStrRewrites.end(), rewritesSort);
-  for (auto it = AsmStrRewrites.begin(); it != AsmStrRewrites.end(); ++it) {
-    const AsmRewrite &AR = *it;
+  for (auto I = AsmStrRewrites.begin(), E = AsmStrRewrites.end(); I != E; ++I) {
+    const AsmRewrite &AR = *I;
     // Check if this has already been covered by another rewrite...
     if (AR.Done)
       continue;
@@ -7518,7 +7518,7 @@ bool MasmParser::parseMSInlineAsm(
         SMLoc OffsetLoc = SMLoc::getFromPointer(AR.IntelExp.OffsetName.data());
         size_t OffsetLen = OffsetName.size();
         auto rewrite_it = std::find_if(
-            it, AsmStrRewrites.end(), [&](const AsmRewrite &FusingAR) {
+            I, AsmStrRewrites.end(), [&](const AsmRewrite &FusingAR) {
               return FusingAR.Loc == OffsetLoc && FusingAR.Len == OffsetLen &&
                      (FusingAR.Kind == AOK_Input ||
                       FusingAR.Kind == AOK_CallInput);
diff --git a/llvm/lib/MC/XCOFFObjectWriter.cpp b/llvm/lib/MC/XCOFFObjectWriter.cpp
index 03a4ee831d895..122f19367ae7b 100644
--- a/llvm/lib/MC/XCOFFObjectWriter.cpp
+++ b/llvm/lib/MC/XCOFFObjectWriter.cpp
@@ -1385,11 +1385,12 @@ void XCOFFObjectWriter::addExceptionEntry(
 unsigned XCOFFObjectWriter::getExceptionSectionSize() {
   unsigned EntryNum = 0;
 
-  for (auto it = ExceptionSection.ExceptionTable.begin();
-       it != ExceptionSection.ExceptionTable.end(); ++it)
+  for (auto I = ExceptionSection.ExceptionTable.begin(),
+            E = ExceptionSection.ExceptionTable.end();
+      I != E; ++I)
     // The size() gets +1 to account for the initial entry containing the
     // symbol table index.
-    EntryNum += it->second.Entries.size() + 1;
+    EntryNum += I->second.Entries.size() + 1;
 
   return EntryNum * (is64Bit() ? XCOFF::ExceptionSectionEntrySize64
                                : XCOFF::ExceptionSectionEntrySize32);
@@ -1397,11 +1398,12 @@ unsigned XCOFFObjectWriter::getExceptionSectionSize() {
 
 unsigned XCOFFObjectWriter::getExceptionOffset(const MCSymbol *Symbol) {
   unsigned EntryNum = 0;
-  for (auto it = ExceptionSection.ExceptionTable.begin();
-       it != ExceptionSection.ExceptionTable.end(); ++it) {
-    if (Symbol == it->second.FunctionSymbol)
+  for (auto I = ExceptionSection.ExceptionTable.begin(),
+            E = ExceptionSection.ExceptionTable.end();
+      I != E; ++I) {
+    if (Symbol == I->second.FunctionSymbol)
       break;
-    EntryNum += it->second.Entries.size() + 1;
+    EntryNum += I->second.Entries.size() + 1;
   }
   return EntryNum * (is64Bit() ? XCOFF::ExceptionSectionEntrySize64
                                : XCOFF::ExceptionSectionEntrySize32);
@@ -1667,17 +1669,18 @@ void XCOFFObjectWriter::writeSectionForDwarfSectionEntry(
 void XCOFFObjectWriter::writeSectionForExceptionSectionEntry(
     const MCAssembler &Asm, ExceptionSectionEntry &ExceptionEntry,
     uint64_t &CurrentAddressLocation) {
-  for (auto it = ExceptionEntry.ExceptionTable.begin();
-       it != ExceptionEntry.ExceptionTable.end(); it++) {
+  for (auto I = ExceptionEntry.ExceptionTable.begin(),
+          E = ExceptionEntry.ExceptionTable.end();
+     I != E; ++I) {
     // For every symbol that has exception entries, you must start the entries
     // with an initial symbol table index entry
-    W.write<uint32_t>(SymbolIndexMap[it->second.FunctionSymbol]);
+    W.write<uint32_t>(SymbolIndexMap[I->second.FunctionSymbol]);
     if (is64Bit()) {
       // 4-byte padding on 64-bit.
       W.OS.write_zeros(4);
     }
     W.OS.write_zeros(2);
-    for (auto &TrapEntry : it->second.Entries) {
+    for (auto &TrapEntry : I->second.Entries) {
       writeWord(TrapEntry.TrapAddress);
       W.write<uint8_t>(TrapEntry.Lang);
       W.write<uint8_t>(TrapEntry.Reason);

github-actions · 2024-07-09T05:55:17Z

✅ With the latest revision this PR passed the C/C++ code formatter.

s-barannikov

(nit) We don't usually end the subject line of a commit message with a period.

MaskRay · 2024-07-09T06:35:05Z

llvm/lib/MC/XCOFFObjectWriter.cpp

@@ -1385,23 +1385,25 @@ void XCOFFObjectWriter::addExceptionEntry(
 unsigned XCOFFObjectWriter::getExceptionSectionSize() {
  unsigned EntryNum = 0;

-  for (auto it = ExceptionSection.ExceptionTable.begin();
-       it != ExceptionSection.ExceptionTable.end(); ++it)
+  for (auto I = ExceptionSection.ExceptionTable.begin(),


this can use for each

Rewritten to for_each

chestnykh · 2024-07-09T06:35:18Z

(nit) We don't usually end the subject line of a commit message with a period.

Got it

nikic

Can't some/all of these be range based for loops instead?

chestnykh · 2024-07-09T07:35:05Z

Can't some/all of these be range based for loops instead?

Is range based loop preferable?

Can't some/all of these be range based for loops instead?

I left comments where the loop can be transformed into range-based. Other loops use iterator in their body so i think we can leave them without changes.

nikic · 2024-07-09T07:38:54Z

Can't some/all of these be range based for loops instead?

Is range based loop preferable?

Yes :) From the same guide: https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible

llvm/lib/MC/XCOFFObjectWriter.cpp

In accordance with https://llvm.org/docs/CodingStandards.html we shouldn't do this. Use range-based loops where possible

In accordance with https://llvm.org/docs/CodingStandards.html we shouldn't do this.

s-barannikov

LGTM
A note for the future: amending commits and force pushing are discouraged, see
https://llvm.org/docs/GitHub.html#updating-pull-requests

chestnykh · 2024-07-12T07:45:25Z

LGTM A note for the future: amending commits and force pushing are discouraged, see https://llvm.org/docs/GitHub.html#updating-pull-requests

FYI i don't have commit access so please commit my PR when it is ready

https://llvm.org/docs/CodingStandards.html tells us that we should avoid evaluating `.end()` each time if possible.

llvmbot added the mc Machine (object) code label Jul 9, 2024

chestnykh force-pushed the optimize-loops-in-MC branch from d2d3977 to 13bafeb Compare July 9, 2024 05:56

chestnykh changed the title ~~Optimize loops in MC.~~ [MC] Optimize loops in MC. Jul 9, 2024

s-barannikov approved these changes Jul 9, 2024

View reviewed changes

chestnykh force-pushed the optimize-loops-in-MC branch from 13bafeb to 20032d3 Compare July 9, 2024 06:34

MaskRay reviewed Jul 9, 2024

View reviewed changes

chestnykh force-pushed the optimize-loops-in-MC branch 2 times, most recently from b7b980e to fdf9be2 Compare July 9, 2024 07:04

nikic reviewed Jul 9, 2024

View reviewed changes

chestnykh force-pushed the optimize-loops-in-MC branch from fdf9be2 to 6d4ae81 Compare July 9, 2024 07:35

s-barannikov self-requested a review July 9, 2024 07:42

chestnykh force-pushed the optimize-loops-in-MC branch from 6d4ae81 to 659626a Compare July 9, 2024 07:47

s-barannikov reviewed Jul 9, 2024

View reviewed changes

llvm/lib/MC/XCOFFObjectWriter.cpp Show resolved Hide resolved

chestnykh force-pushed the optimize-loops-in-MC branch from 659626a to 4b9c1ef Compare July 9, 2024 07:57

chestnykh added 2 commits July 9, 2024 10:59

[MC,COFF] Don't evaluate end() every iteration

222b654

In accordance with https://llvm.org/docs/CodingStandards.html we shouldn't do this. Use range-based loops where possible

[MC,AsmParser] Don't evaluate end() every iteration

4b9c1ef

In accordance with https://llvm.org/docs/CodingStandards.html we shouldn't do this.

s-barannikov approved these changes Jul 9, 2024

View reviewed changes

nikic merged commit 345861b into llvm:main Jul 12, 2024
5 of 7 checks passed

aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024

[MC] Optimize loops in MC (llvm#98114)

e5a7b0a

https://llvm.org/docs/CodingStandards.html tells us that we should avoid evaluating `.end()` each time if possible.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[MC] Optimize loops in MC. #98114

[MC] Optimize loops in MC. #98114

Uh oh!

chestnykh commented Jul 9, 2024

Uh oh!

llvmbot commented Jul 9, 2024

Uh oh!

github-actions bot commented Jul 9, 2024 •

edited

Loading

Uh oh!

s-barannikov left a comment

Uh oh!

MaskRay Jul 9, 2024

Uh oh!

chestnykh Jul 9, 2024

Uh oh!

chestnykh commented Jul 9, 2024

Uh oh!

nikic left a comment

Uh oh!

chestnykh commented Jul 9, 2024

Uh oh!

nikic commented Jul 9, 2024

Uh oh!

Uh oh!

s-barannikov left a comment

Uh oh!

chestnykh commented Jul 12, 2024

Uh oh!

Uh oh!

Uh oh!

[MC] Optimize loops in MC. #98114

[MC] Optimize loops in MC. #98114

Uh oh!

Conversation

chestnykh commented Jul 9, 2024

Uh oh!

llvmbot commented Jul 9, 2024

Uh oh!

github-actions bot commented Jul 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

s-barannikov left a comment

Choose a reason for hiding this comment

Uh oh!

MaskRay Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

chestnykh Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

chestnykh commented Jul 9, 2024

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

chestnykh commented Jul 9, 2024

Uh oh!

nikic commented Jul 9, 2024

Uh oh!

Uh oh!

s-barannikov left a comment

Choose a reason for hiding this comment

Uh oh!

chestnykh commented Jul 12, 2024

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 9, 2024 •

edited

Loading