ebevhan added inline comments.
================ Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:1119 + case ISD::SSAT: + // Target legalization checked here? + Action = TargetLowering::Expand; ---------------- leonardchan wrote: > ebevhan wrote: > > leonardchan wrote: > > > ebevhan wrote: > > > > leonardchan wrote: > > > > > @ebevhan Do you know if this is where checking if this intrinsic is > > > > > supported by the target should be? > > > > Yes, this would be it. For nodes like ssat and fixed-point ops, you > > > > could ask a custom target hook, maybe > > > > `TLI.getScaledOperationAction(<opcode>, <type>, <scale/sat_bw>)` to > > > > determine the legality of a given node. > > > > > > > > When type-legalizing you also need to handle things for specific nodes > > > > as well, since things behave differently when you legalize > > > > operands/results on different nodes. > > > > > > > > (Of course, this all depends on whether we do the legalization here or > > > > in IR instead.) > > > Made the default action `Expand` for this in `TargetLoweringBase` and > > > targets can override this themselves. > > > > > > I think ssat may not need a custom hook since I don't think it requires > > > anything extra like fixsmul if it has to be expanded. But I see what you > > > mean now by how different nodes may require different steps for > > > legalization. > > You don't need an extra hook to determine anything extra for expansion, but > > you do need the hook to determine if it's legal in the first place. As I > > mentioned in another comment, an i16 ssat with a saturating width of 8 > > might be legal, but an i16 ssat with saturating width 12 might not. The > > former must not be expanded, but the latter must. > Got it. Just to clarify though, legalizations regarding the scale would only > apply for our mul/div intrinsics right? Operations like saturation and sat > add/sub which don't care about the scale do not need this hook right, since > it would also be useful for these operations to work on regular integers? Saturating add/sub won't need anything, but I still think saturation requires some extra check to determine if an operation with a specific saturating bitwidth should be legal or expanded. As I mentioned, a saturation with a type might be legal for one saturating bitwidth but not for another. Repository: rL LLVM https://reviews.llvm.org/D52286 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
