[PATCH] D41330: [X86] Reduce Store Forward Block issues in HW

Lama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 8 12:37:53 PST 2018


lsaba added inline comments.


================
Comment at: lib/Target/X86/X86FixupSFB.cpp:127
+
+std::map<unsigned, std::vector<unsigned>> PotentialBlockedMemCpy{
+    {X86::MOVUPSrm, {X86::MOVUPSmr, X86::MOVAPSmr}},
----------------
craig.topper wrote:
> craig.topper wrote:
> > Should these be DenseMaps? @RKSimon, what do you think?
> > 
> > Can you use a std::array or std::pair instead of std::vector for the second type since the size is fixed. Each std::vector will be a separate heap allocation.
> I don't think the DenseMap question here was answered.
I think DenseMap is less suitable since it doesn't have a static initializer constructor


================
Comment at: lib/Target/X86/X86FixupSFB.cpp:449
+             *LoadInst->getParent()->getParent())) /
+         8;
+}
----------------
craig.topper wrote:
> Can you clang-format at least this function? That 8 looks really out of place. So does the splitting of the getRegClass call.
This weird format is actually clang-format's output, I changed it to make it more readable


https://reviews.llvm.org/D41330





More information about the llvm-commits mailing list