Page MenuHomePhabricator

[CodeGen] Error on unsupported checked multiplies early
AbandonedPublic

Authored by vsk on Oct 12 2017, 3:26 PM.

Details

Summary

LLVM's smul.with.overflow intrinsic isn't supported on X86 for bit
widths larger than 64, or on X86-64 for bit widths larger than 128.

The failure mode is either a linker error ("the __muloti4 builtin isn't
available for this target") or an assertion failure ("SelectionDAG
doesn't know what builtin to call").

Until we actually add builtin support for 128-bit multiply-with-overflow
on X86, we should error-out on unsupported calls as early as possible.

https://bugs.llvm.org/show_bug.cgi?id=34920

Diff Detail

Event Timeline

vsk created this revision.Oct 12 2017, 3:26 PM
rjmccall added inline comments.Oct 12 2017, 3:33 PM
lib/CodeGen/CGBuiltin.cpp
2272

Is there a reason this only fails on x86? If LLVM doesn't have generic wide-operation lowering, this probably needs to be a target whitelist rather than a blacklist.

vsk updated this revision to Diff 118857.Oct 12 2017, 3:59 PM
  • Update to check against a whitelist of supported targets.
vsk added inline comments.Oct 12 2017, 4:00 PM
lib/CodeGen/CGBuiltin.cpp
2272

That makes sense. For the 128-bit operation, the whitelist is {x86-64, wasm64, mips64}. We don't support this operation for bit widths larger than 128 bits on any target. I'll update the patch accordingly.

rjmccall edited edge metadata.Oct 12 2017, 4:01 PM

Okay. Sounds good to me.

We intend to actually implement the generic lowering, I hope?

vsk added a comment.Oct 12 2017, 4:03 PM

Okay. Sounds good to me.

We intend to actually implement the generic lowering, I hope?

Yes, I'll make a note of that in PR34920, and am tracking the bug internally in rdar://problem/34963321.

joerg added a subscriber: joerg.Oct 12 2017, 6:32 PM
joerg added inline comments.
lib/CodeGen/CGBuiltin.cpp
2272

That sounds wrong. __int128_t should be supported by all 64bit architectures, not just those three.

vsk added inline comments.Oct 12 2017, 6:35 PM
lib/CodeGen/CGBuiltin.cpp
2272

I didn't mean to imply generic int128_t operations aren't supported. This patch just focuses on muloti4 (signed multiplication with overflow checking).

vsk abandoned this revision.Dec 12 2017, 5:57 PM

Abandoned in favor of a proper fix: https://reviews.llvm.org/D41149