[PATCH] D64845: [GlobalISel] Check LLT size matches memory size for non-truncating stores.

Wed Jul 17 11:24:59 PDT 2019

aemerson marked an inline comment as done.
aemerson added inline comments.

================
Comment at: llvm/utils/TableGen/GlobalISelEmitter.cpp:3314
+        InsnMatcher.addPredicate<MemoryVsLLTSizePredicateMatcher>(
+            0, MemoryVsLLTSizePredicateMatcher::EqualTo, 0);
+      }
----------------
arsenm wrote:
> arsenm wrote:
> > aemerson wrote:
> > > arsenm wrote:
> > > > Should also continue? Could make this a select  on the match type to the one addPredicate call
> > > Yeah, just seen this doesn't work. I'm not familiar with this code, but how do non-truncstore predicates end up on trunc stores such that it breaks?
> > I don't exactly understand what the expected relationship between predicate bits set on a PatFrag and the PatFrags which used it is.
> > 
> > It's possible something is wrong with the AMDGPU PatFrags. This system is pretty confusing, because each property is added in a different layer of PatFrags defined in TargetSelectionDAG.td. The AMDGPU PatFrags reproduce the same hierarchy, with the additional address space predicates.
> > 
> > There is also the extra IsTruncStore bit which needs to be manually set on the base truncstore PatFrag. For some reason the derivative PatFrags do not have it set, as it should be implied. However, they still need to set IsStore, although I would expect these to both behave the same way.
> I'm also not sure the way this loop is structured makes sense. It seems to assume too much about what different predicates can be combined. The atomic predicates also don't work correctly
@dsanders any idea what to do?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64845