[PATCH] D67017: [X86] Add initial support for unfolding broadcast loads from arithmetic instructions to enable LICM hoisting of the load

[Craig Topper via Phabricator via llvm-commits]

craig.topper marked an inline comment as done.
craig.topper added a comment.

In D67017#1653603 <https://reviews.llvm.org/D67017#1653603>, @RKSimon wrote:

> Does this break the X86FoldTablesEmitter in anyway?
>
> In the future do you think we could we add something similar to support scalar ops and ops where memory size != register size?


I don't think it will break the X86FoldTablesEmitter. The emitter just won't generate the new table.

Yes I think we could support scalar ops in the future.



================
Comment at: llvm/lib/Target/X86/X86InstrFoldTables.cpp:5248
 
+static const X86MemoryFoldTableEntry BroadcastFoldTable2[] = {
+  { X86::VADDPDZ128rr,   X86::VADDPDZ128rmb,   TB_BCAST_SD },
----------------
RKSimon wrote:
> Do masked ops work? Possibly add a few to this initial test?
I think if the mask was dynamically all 0 then any memory fault would be masked. So I think if we unfold it, we would need to generate a masked broadcast as well to maintain the fault suppression. Which wouldn't be eligible for hoisting. Though we don't fold masked operations in the first place so unfolding without applying the mask would be fine today, but I wouldn't want it to break in the future.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67017/new/

https://reviews.llvm.org/D67017