rjmccall added inline comments.
================ Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3444 case BO_NE: + return Builder.CreateICmpNE(FullLHS, FullRHS); + case BO_Mul: ---------------- leonardchan wrote: > rjmccall wrote: > > leonardchan wrote: > > > rjmccall wrote: > > > > Are padding bits guaranteed zero or unspecified? Or are we just not > > > > really supporting padding bits all the way to IRGen at this time? > > > I believe the consensus was leaving them unspecified, so operations that > > > can cause overflow into them would result in undefined behavior. > > If I'm understanding you correctly, you're saying that (1) we are assuming > > that inputs are zero-padded and (2) we are taking advantage of the > > non-saturating-overflow-is-UB rule to avoid clearing the padding bits after > > arithmetic. That's actually what I meant by "guaranteed zero", so we have > > our labels reversed, but at least we understand each other now. > > > > (I suppose you're thinking of this as "unspecified" because non-saturating > > arithmetic can leave an unspecified value there. I think of this as a > > "guarantee" because it's basically a representational invariant: it's a > > precondition for correctness that the bit is zero, and all operations > > guarantee that the bit will be zero in their well-defined cases (but > > overflow is not well-defined). Just a difference in perspective, I guess.) > > > > Is this written down somewhere? Are there targets that use the opposite > > ABI rule that we might need to support someday? > > > > At any rate, I think it's worth adding a short comment here explaining why > > it's okay to do a normal comparison despite the possibility of padding > > bits. Along those lines, is there any value in communicating to the > > backend that padded-unsigned comparisons could reasonably be done with > > either a signed or unsigned comparison? Are there interesting targets > > where one or the other is cheaper? > Yes. We assume inputs are zero padded, and that non-saturating overflow is > undefined so we do not need to clear the padding after writing a new value. > Sorry for the misunderstanding. I see what you mean by guaranteed zero. > > Overflow in general is controlled by the `FX_FRACT_OVERFLOW` and > `FX_ACCUM_OVERFLOW` pragmas, where if these are set to `DEFAULT`, operations > that can overflow with these types is undefined according to the standard > (4.1.3). In terms of padding bits, clause 6.2.6.3 mentions that the values of > padding bits are "unspecified". I imagine this means that we can assume the > value to be whatever we want it to, so we can assume that these values are a > guaranteed zero. > > I believe @ebevhan requested this being added since his downstream > implementation used padding to match the scales of signed and unsigned types, > so he may be able to offer more information on targets with different ABIs. > We don't plan to use any special hardware, so we're following the "typical > desktop processor" layout that uses the whole underlying int and no padding > (mentioned in Annex 3). > > In the same section, the standard also mentions types for other targets that > may have padding, so there could be some value in indicating to the backend > that for these particular targets, this part of the operand is padding, so > select an appropriate operation that performs a comparison on this size type. > But I don't know much about these processors and would just be guessing at > how they work. Okay. If we ever have a target that uses an ABI that (1) includes padding but (2) considers it to be "unspecified" in the sense of "it can validly be non-zero", we'll have to change this code, but I agree it's the right move to not preemptively generalize to cover that possibility. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57219/new/ https://reviews.llvm.org/D57219 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
