[PATCH] D46018: [GlobalISel][IRTranslator] Split aggregates during IR translation

Roman Tereshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 14 21:51:02 PDT 2018


rtereshin added a comment.

In https://reviews.llvm.org/D46018#1098606, @aemerson wrote:

> Thanks. -time-passes shows that IRTranslator is now using around 2MB. It's also taking most of the compile time vs other passes like your initial analysis showed, but we trade off the legalizer doing much less work.
>
> I haven't run this version of the patch through much testing yet.


Looks like there is a higher precision way of measuring the memory consumption for the whole process on macOS: `time -l`. Given that with this specific test IRTranslator was allocating much more memory than anything else, I think it also makes sense to measure the entire `llc` compile from outside:

`command time -l ./bin/llc -O0 -global-isel=true -global-isel-abort=2 -mtriple aarch64-- test.ll -o /dev/null -time-passes`

And the results are great:

[GlobalISel Before]
49.5M / 68ms

[FastISel / SelectionDAG ISel]
57.3M / 7,250ms

[GlobalISel After / Original Patch]
337.7M / 740ms

[GlobalISel After / Updated Patch]
29.5M / 40ms

(memory figure is "maximum resident set size" as shown by `time -l`, the time figure is "User+System / Total" as shown by `-time-passes`)

I think it's a huge win and we can stop here as far as performance and memory consumption are concerned. Thank you for the update!



================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:423
+
+    MachinePointerInfo Ptr(LI.getPointerOperand(), Offsets[i]);
+    unsigned BaseAlign = getMemOpAlignment(LI);
----------------
`Offsets[i]` here needs to be in bytes as well. 


================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:427
+        Ptr, Flags, (MRI->getType(Regs[i]).getSizeInBits() + 7) / 8,
+        MinAlign(BaseAlign, Offsets[i]), AAMDNodes(), nullptr,
+        LI.getSyncScopeID(), LI.getOrdering());
----------------
Ditto, `MinAlign` expects everything in bytes as far as I can tell.


================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:452
+
+    MachinePointerInfo Ptr(SI.getPointerOperand(), Offsets[i]);
+    unsigned BaseAlign = getMemOpAlignment(SI);
----------------
Ditto, `MachinePointerInfo` expects everything in bytes.


================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:456
+        Ptr, Flags, (MRI->getType(Vals[i]).getSizeInBits() + 7) / 8,
+        MinAlign(BaseAlign, Offsets[i]), AAMDNodes(), nullptr,
+        SI.getSyncScopeID(), SI.getOrdering());
----------------
Ditto.


Repository:
  rL LLVM

https://reviews.llvm.org/D46018





More information about the llvm-commits mailing list