Quuxplusone added a comment. In D114425#3216802 <https://reviews.llvm.org/D114425#3216802>, @majnemer wrote:
> OOC, how hard would it be to generalize this builtin a little? It is nice > that we have builtins like `__builtin_add_overflow` which do the right thing > regardless of their input. > > It seems like it would be nice if we started to expose more intrinsics which > did the right thing regardless of operand width; another bonus is that it > composes well with language features like `_BitInt`. IMHO such builtins are nice iff the programmer can be 100% sure that the compiler will interpret them the same way as a human reader. `__builtin_add_overflow` is easy because its first two arguments are "mathematical integers" (where integer promotion doesn't matter) and its third argument is a pointer (where integer promotion can't happen). So you can really throw any combination of types at it, and it'll do "the right thing" https://godbolt.org/z/sa7b894oa (although I admit I was surprised that this worked). For a hypothetical `__builtin_bswap`, you would probably need a similar pointer-based interface like short s16 = 0xFEDC; __builtin_bswap(&s16); // hypothetically assert(s16 == 0xDCFE); assert(__builtin_bswap16(s16) == 0x0000DCFE); assert(__builtin_bswap32(s16) == 0xDCFEFFFF); // the problem to solve: s16 eagerly promotes to int, which changes the result The downside is that the pointer-based interface is less ergonomic than today's value-based signatures, and probably worse codegen at `-O0` (because the programmer has to materialize the operand into a named variable, and then the compiler won't remove that variable because you might want to debug it). The upside (as you said) is that a generic builtin could work with `_ExtInt` types and so on. ================ Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2930 + } + [[fallthrough]]; case Builtin::BI__builtin_bswap16: ---------------- Re clang-format's complaint: I would either move `[[fallthrough]];` inside the curly braces, or (probably better) just eliminate the fallthrough by either duplicating line 2934 or else doing ``` case Builtin::BI__builtin_bswap64: case Builtin::BI__builtin_bswap128: { if (BuiltinIDIfNoAsmLabel == Builtin::BI__builtin_bswap128 && !Target.hasInt128Type()) CGM.ErrorUnsupported(E, "__builtin_bswap128"); return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::bswap)); } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114425/new/ https://reviews.llvm.org/D114425 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
