On 6/9/20 1:43 AM, Richard Biener wrote:
On Mon, Jun 8, 2020 at 11:59 PM Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:PR 95581 reports an ICE in a call to gimple_call_arg() with an out-of-bounds argument number (2). The attached patch avoids the ICE but I'm not sure it's the right fix. Other than verifying the ICE is gone with a powerpc64 cross-compiler I couldn't come up with a generic test cases to exercise this change. The call, synthesized by the vectorizer, is vect__5.6_24 = __builtin_altivec_mask_for_load (vectp_a.5_8); but the function is declared by the powerpc64 back end to take two arguments: long and const void*. Is it correct for the builtin to be called with fewer arguments than their type indicates? (If it isn't the patch shouldn't be necessary but some other fix would be needed.)I think the backend declaration is wrong, the function only takes a void * argument and returns a long.
Thanks. In his comment on the bug Segher (CC'd) points to the internals manual that documents the function: https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/target.def;h=07059a87caf7cc0237d583124b476ee45ea41ed5;hb=HEAD#l1744 (By the way, thanks for the pointer!) If I read it right, ihe function f in the TARGET_VECTORIZE_BUILTIN_MASK_FOR_LOAD documentation is __builtin_altivec_mask_for_load. Although the manual isn't unequivocal about this but it does suggest the address addr the function is given as an argument should be the first (and presumably only) argument. This matches the call in the IL where the first argument is a pointer, but not the function's signature. I think the middle end needs to be able to rely on built-in function types when processing calls: i.e., the types and numbers of actual arguments in the calls need to match those of the built-in function type. Otherwise it would have to validate every call against the function signature and avoid treating it as a built-in if it doesn't match. There are parts of the middle end that do that for library built-ins (because they can be declared in a program with mismatched signatures) but as we have seen it's error-prone. I don't think it would be helpful to try to extend this approach to internal built-ins. So I agree that the real problem is the declaration of the built-in. I have no issue with also committing the proposed patch in the meantime, until the back end is fixed, if it helps. But I'll leave that decision to the middle end maintainers. Martin
