[PATCH] D100691: [TableGen] Store predicates in PatternToMatch as ListInit *. Add string for HwModeFeatures

[Krzysztof Parzyszek via Phabricator via llvm-commits]

kparzysz added inline comments.


================
Comment at: llvm/utils/TableGen/CodeGenDAGPatterns.cpp:4370
-
-      // Add negations of the HM's predicates to the default predicate.
-      DefaultPred.emplace_back(Predicate(HM.Features, false));
----------------
Is this part removed in your patch?  The "default" case could be vacuously true if there are no conditions attached to it, and it could be applied when a more specialized case should be applied instead.
The "default" was originally invented for situations where the specific modes don't cover all possibilities when combined.  I guess in practice the "default" ends up being one of the explicitly defined modes, so this "accidental application" may not be happening.


================
Comment at: llvm/utils/TableGen/CodeGenDAGPatterns.cpp:1477
+    PredicateCheck += HwModeFeatures;
+    PredicateCheck += "\"))";
   }
----------------
Paul-C-Anagnostopoulos wrote:
> craig.topper wrote:
> > Paul-C-Anagnostopoulos wrote:
> > > Any reason you didn't combine the catenations with '+' ?
> > Concatenating them would produce a temporary std::string that would need a heap allocation. That would then need to be copied into the SmallString. If the total string is small enough the SmallString won't heap allocate. If we copy each piece in separately we avoid heap allocations for temporaries.
> Sounds like there's room for an optimization that looks at the complete sequence of catenations to decide whether there is room in the SmallString. But good to know.
Isn't `Twine` specifically designed to optimize concatenations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100691