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

Lama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 20 09:23:23 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}},
----------------
chandlerc wrote:
> lsaba wrote:
> > 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
> But this still isn't going to be a constant. It isn't marked constexpr. This will require building a map every time the binary starts.
> 
> Please don't create globals with complex initializers in LLVM:
> http://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors
> 
> Also, std::map is *really* slow...
> 
> Why not just build a function with a switch to do this mapping?
> 
> Even better, could this be generated by tablegen?
back to using switch functions for this.



https://reviews.llvm.org/D41330





More information about the llvm-commits mailing list