rjmccall added a comment. In D82663#2144219 <https://reviews.llvm.org/D82663#2144219>, @ebevhan wrote:
> In D82663#2142426 <https://reviews.llvm.org/D82663#2142426>, @rjmccall wrote: > > > Would it be sensible to use a technical design more like what the matrix > > folks are doing, where LLVM provides a small interface for emitting > > operations with various semantics? FixedPointSemantics would move to that > > header, and Clang would just call into it. That way you get a lot more > > flexibility in how you generate code, and the Clang IRGen logic is still > > transparently correct. If you want to add intrinsics or otherwise change > > the IR patterns used for various operations, you don't have to rewrite a > > bunch of Clang IRGen logic every time, you just have to update the tests. > > It'd then be pretty straightforward to have internal helper functions in > > that interface for computing things like whether you should use signed or > > unsigned intrinsics given the desired FixedPointSemantics. > > > This seems like a reasonable thing to do for other reasons as well. Also > moving the actual APFixedPoint class to LLVM would make it easier to reuse > the fixedpoint calculation code for constant folding in LLVM, for example. Just to say "I told you so", I'm pretty sure I told people this would happen. :) >> My interest here is mainly in (1) keeping IRGen's logic as obviously correct >> as possible, (2) not hard-coding a bunch of things that really feel like >> workarounds for backend limitations, and (3) not complicating core >> abstractions like FixedPointSemantics with unnecessary extra rules for >> appropriate use, like having to pass an extra "for codegen" flag to get >> optimal codegen. If IRGen can just pass down the high-level semantics it >> wants to some library that will make intelligent decisions about how to emit >> IR, that seems best. > > Just to clarify something here; would the interface in LLVM still emit signed > operations for unsigned with padding? If that's the best IR pattern to emit, yes. > If so, why does dealing with the padding bit detail in LLVM rather than Clang > make more sense? Because frontends should be able to just say "I have a value of a type with these semantics, I need you to do these operations, go do them". The whole purpose of this interface would be to go down a level of abstraction by picking the best IR to represent those operations. Maybe we're not in agreement about what this interface looks like — I'm imagining something like struct FixedPointEmitter { IRBuilder &B; FixedPointEmitter(IRBuilder &B) : B(B) {} Value *convert(Value *src, FixedPointSemantics srcSemantics, FixedPointSemantics destSemantics); Value *add(Value *lhs, FixedPointSemantics lhsSemantics, Value *rhs, FixedPointSemantics rhsSemantics) }; > The regular IRBuilder is relatively straightforward in its behavior. I > suspect that if anything, LLVM would be equally unwilling to take to take > IRBuilder patches that emitted signed intrinsics for certain unsigned > operations only due to a detail in Embedded-C's implementation of fixedpoint > support. Most things in IRBuilder don't have variant representations beyond what's expressed by the value type. The fact that we've chosen to do so here necessitates a more complex interface. > Regarding backend limitations, I guess I could propose an alternate solution. > If we change FixedPointSemantics to strip the padding bit for both saturating > and nonsaturating operations, it may be possible to detect in isel that the > corresponding signed operation could be used instead when we promote the type > of an unsigned one. For example, if we emit i15 umul.fix scale 15, we could > tell in lowering that i16 smul.fix scale 15 is legal and use that instead. > Same for all the other intrinsics, including the non-fixedpoint > uadd.sat/usub.sat. > > The issue with this approach (which is why I didn't really want to do it) is > that it's not testable. No upstream target has these intrinsics marked as > legal. I doubt anyone would accept a patch with no tests. > It may also be less efficient than just emitting the signed operations in > the first place, because we are forced to trunc and zext in IR before and > after every operation. I don't want to tell you the best IR to use to get good code; I just want frontends to have a reasonably canonical interface to use that matches up well with the information we have. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82663/new/ https://reviews.llvm.org/D82663 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
