leonardchan added inline comments.
================ Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3603-3604 auto ResultFixedSema = Ctx.getFixedPointSemantics(ResultTy); - auto CommonFixedSema = LHSFixedSema.getCommonSemantics(RHSFixedSema, true); + auto CommonFixedSema = LHSFixedSema.getCommonSemantics( + IsShift ? LHSFixedSema : RHSFixedSema, true); ---------------- Could this instead be: ``` auto CommonFixedSema = IsShift ? LHSFixedSema : LHSFixedSema.getCommonSemantics(RHSFixedSema, true); ``` ================ Comment at: clang/test/Frontend/fixed_point_compound.c:384 + // CHECK-NEXT: [[TMP4:%.*]] = load i16, i16* @suf, align 2 + // CHECK-NEXT: [[TMP5:%.*]] = trunc i32 [[TMP3]] to i16 + // SIGNED-NEXT: [[TMP6:%.*]] = call i16 @llvm.ushl.sat.i16(i16 [[TMP4]], i16 [[TMP5]]) ---------------- This seems very unlikely, but if `i` in this case was a value at least 2^16, the upper half would be cut off and we'd potentially shift by an improper amount for some values of `i` when we should actually clamp to the max value. We should probably still find the common semantics for both sides if we're doing a shift. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83294/new/ https://reviews.llvm.org/D83294 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
