From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 93697 invoked by alias); 14 Nov 2018 16:35:36 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 93682 invoked by uid 89); 14 Nov 2018 16:35:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sk:aldyh@r, wide_int, aldyh@redhat.com, sk:aldyhr X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Nov 2018 16:35:34 +0000 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F3CAA2D806 for ; Wed, 14 Nov 2018 16:35:32 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-68.rdu2.redhat.com [10.10.112.68]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0C3E110E2B46 for ; Wed, 14 Nov 2018 16:35:31 +0000 (UTC) From: Jeff Law Openpgp: preference=signencrypt To: gcc-patches Subject: [committed] Fix rl78 newlib build failure due to bogus operand_subword_force argument Message-ID: Date: Wed, 14 Nov 2018 16:35:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------0EAAADF0D6F93CB29E8B6C20" X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg01301.txt.bz2 This is a multi-part message in MIME format. --------------0EAAADF0D6F93CB29E8B6C20 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-length: 5726 For the attached testcase compiled with -O2 -mg10 on rl78 port we get into expand_builtin_signbit. EXP is exactly what you'd expect. A CALL_EXPR to __builtin_signbit with an argument like: arg:0 unit-size align:16 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type 0x7fffe9e21498 precision:32 pointer_to_this > visited var def_stmt GIMPLE_NOP version:2> TARGET is basically what you'd expect as well: (gdb) p debug_rtx (target) (reg:HI 43 [ _1 ]) There is no suitable optab and we get into this code: if (GET_MODE_SIZE (fmode) <= UNITS_PER_WORD) { imode = int_mode_for_mode (fmode).require (); temp = gen_lowpart (imode, temp); } else { imode = word_mode; /* Handle targets with different FP word orders. */ if (FLOAT_WORDS_BIG_ENDIAN) word = (GET_MODE_BITSIZE (fmode) - bitpos) / BITS_PER_WORD; else word = bitpos / BITS_PER_WORD; temp = operand_subword_force (temp, word, fmode); bitpos = bitpos % BITS_PER_WORD; } rl78 has 8 bit words. So we get into the else clause and call operand_subword_force with (gdb) p debug_rtx (temp) (reg/v:SF 45 [ fp ]) $15 = void (gdb) p word $16 = 3 (gdb) p fmode $17 = {m_mode = E_SFmode} Which generally makes sense. That returns: (subreg:QI (reg/v:SF 45 [ fp ]) 3) And bitpos will be set to 7. ie, check the sign bit, exactly what we should expect. We copy the subreg expression into a pseudo: (reg:QI 46) Remember that we wanted the result in HImode. So we then get into this conditional: if (bitpos < GET_MODE_BITSIZE (rmode)) { wide_int mask = wi::set_bit_in_zero (bitpos, GET_MODE_PRECISION (rmode)); if (GET_MODE_SIZE (imode) > GET_MODE_SIZE (rmode)) temp = gen_lowpart (rmode, temp); temp = expand_binop (rmode, and_optab, temp, immed_wide_int_const (mask, rmode), NULL_RTX, 1, OPTAB_LIB_WIDEN); } Which again makes perfect sense. We want to do a bit-wise AND in HImode of the pseudo and the mask. So far, so good. The target doesn't have HImode logicals. But they can be synthesized from QImode logicals by this loop in expand_binop: /* These can be done a word at a time. */ if ((binoptab == and_optab || binoptab == ior_optab || binoptab == xor_optab) && is_int_mode (mode, &int_mode) && GET_MODE_SIZE (int_mode) > UNITS_PER_WORD && optab_handler (binoptab, word_mode) != CODE_FOR_nothing) [ ... ] /* Do the actual arithmetic. */ for (i = 0; i < GET_MODE_BITSIZE (int_mode) / BITS_PER_WORD; i++) { rtx target_piece = operand_subword (target, i, 1, int_mode); rtx x = expand_binop (word_mode, binoptab, operand_subword_force (op0, i, int_mode), operand_subword_force (op1, i, int_mode), target_piece, unsignedp, next_methods); if (x == 0) break; if (target_piece != x) emit_move_insn (target_piece, x); } This is where things start to get interesting. Notice the calls to operand_subword_force. We'll end up calling operand_subword_force with: Breakpoint 4, operand_subword_force (op=0x7fffe9f148a0, offset=..., mode=E_HImode) at /home/gcc/GIT-3/gcc/gcc/emit-rtl.c:1777 1777 rtx result = operand_subword (op, offset, 1, mode); (gdb) p debug_rtx (op) (reg:QI 46) $28 = void (gdb) p offset $29 = {> = {coeffs = {0}}, } (gdb) p mode $30 = E_HImode INT_MODE as passed into operand_subword_force is documented as the mode to use if the first operand is a constant integer (which are modeless). In theory it shouldn't be used for other operands. But reality is different. operand_subword_force will call operand_subword. operand_subword will eventually call simplify_gen_subreg/simplify_subreg. In that call OUTERMODE will be WORD_MODE, INNERMODE will be that passed in MODE to operands_subword_force (HImode in this case). simplify_subreg is going to hit this assertion: gcc_assert (GET_MODE (op) == innermode || GET_MODE (op) == VOIDmode); Remember OP is a QImode register. INNERMODE is HImode that we passed along just in case OP was a CONST_INT. And boom, we ICE. I've gone back and forth a few times on the best place to fix this. At one time I just simplified this special case in operand_subword so that we'd never have to pass it off to simplify_gen_subreg/simplify_subreg. But that doesn't seem right. I keep coming back to those original calls to operand_subword_force. Blindly passing in INT_MODE there just seems wrong. ISTM we should only be passing in INT_MODE when the operand is a constant. For non-constant operands, we should just pass in VOIDmode. In the case where we end up inside simplify_gen_subreg/simplify_subreg that will result in using the mode of the operand which is precisely what we want. And that's precisely what this patch does. That fixes the ICE and I've verified the assembly code looks right on the rl78 port. Furthermore, newlib will successfully build with this patch. It's been through a full cycle in my tester. So it's been through bootstraps on big/little endian systems, built kernels, built runtimes (glibc, newlib, libgcc) -- essentially covering nearly all our targets to varying degrees. Installing on the trunk. Jeff --------------0EAAADF0D6F93CB29E8B6C20 Content-Type: text/plain; charset=UTF-8; name="Q" Content-Transfer-Encoding: base64 Content-Disposition: inline; filename="Q" Content-length: 2587 ZGlmZiAtLWdpdCBhL2djYy9DaGFuZ2VMb2cgYi9nY2MvQ2hhbmdlTG9nCmlu ZGV4IDNlZmQ5NmI1NzBlLi5iZTc1YzY4NzRjOCAxMDA2NDQKLS0tIGEvZ2Nj L0NoYW5nZUxvZworKysgYi9nY2MvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsOCBA QAorMjAxOC0xMS0xNCAgSmVmZiBMYXcgIDxsYXdAcmVkaGF0LmNvbT4KKwor CSogb3B0YWJzLmMgKGV4cGFuZF9iaW5vcCk6IFBhc3MgSU5UX01PREUgdG8g b3BlcmFuZF9zdWJ3b3JkX2ZvcmNlCisJaWZmIHRoZSBvcGVyYW5kIGlzIGEg Y29uc3RhbnQuCisKIDIwMTgtMTEtMTQgIEFsZHkgSGVybmFuZGV6ICA8YWxk eWhAcmVkaGF0LmNvbT4KIAogCSogZ2ltcGxlLXNzYS1ldnJwLWFuYWx5emUu YwpkaWZmIC0tZ2l0IGEvZ2NjL29wdGFicy5jIGIvZ2NjL29wdGFicy5jCmlu ZGV4IDYwNTIyMjJjOTBjLi5jN2QxZjIyZTdhOCAxMDA2NDQKLS0tIGEvZ2Nj L29wdGFicy5jCisrKyBiL2djYy9vcHRhYnMuYwpAQCAtMTM3NywxMiArMTM3 NywxNCBAQCBleHBhbmRfYmlub3AgKG1hY2hpbmVfbW9kZSBtb2RlLCBvcHRh YiBiaW5vcHRhYiwgcnR4IG9wMCwgcnR4IG9wMSwKICAgICAgIHN0YXJ0X3Nl cXVlbmNlICgpOwogCiAgICAgICAvKiBEbyB0aGUgYWN0dWFsIGFyaXRobWV0 aWMuICAqLworICAgICAgZW51bSBtYWNoaW5lX21vZGUgb3AwX21vZGUgPSBD T05TVEFOVF9QIChvcDApID8gaW50X21vZGUgOiBWT0lEbW9kZTsKKyAgICAg IGVudW0gbWFjaGluZV9tb2RlIG9wMV9tb2RlID0gQ09OU1RBTlRfUCAob3Ax KSA/IGludF9tb2RlIDogVk9JRG1vZGU7CiAgICAgICBmb3IgKGkgPSAwOyBp IDwgR0VUX01PREVfQklUU0laRSAoaW50X21vZGUpIC8gQklUU19QRVJfV09S RDsgaSsrKQogCXsKIAkgIHJ0eCB0YXJnZXRfcGllY2UgPSBvcGVyYW5kX3N1 YndvcmQgKHRhcmdldCwgaSwgMSwgaW50X21vZGUpOwogCSAgcnR4IHggPSBl eHBhbmRfYmlub3AgKHdvcmRfbW9kZSwgYmlub3B0YWIsCi0JCQkJb3BlcmFu ZF9zdWJ3b3JkX2ZvcmNlIChvcDAsIGksIGludF9tb2RlKSwKLQkJCQlvcGVy YW5kX3N1YndvcmRfZm9yY2UgKG9wMSwgaSwgaW50X21vZGUpLAorCQkJCW9w ZXJhbmRfc3Vid29yZF9mb3JjZSAob3AwLCBpLCBvcDBfbW9kZSksCisJCQkJ b3BlcmFuZF9zdWJ3b3JkX2ZvcmNlIChvcDEsIGksIG9wMV9tb2RlKSwKIAkJ CQl0YXJnZXRfcGllY2UsIHVuc2lnbmVkcCwgbmV4dF9tZXRob2RzKTsKIAog CSAgaWYgKHggPT0gMCkKZGlmZiAtLWdpdCBhL2djYy90ZXN0c3VpdGUvQ2hh bmdlTG9nIGIvZ2NjL3Rlc3RzdWl0ZS9DaGFuZ2VMb2cKaW5kZXggNTBlNTNm MGIxOTYuLmNlZTMzNzk2Y2M1IDEwMDY0NAotLS0gYS9nY2MvdGVzdHN1aXRl L0NoYW5nZUxvZworKysgYi9nY2MvdGVzdHN1aXRlL0NoYW5nZUxvZwpAQCAt MSwzICsxLDcgQEAKKzIwMTgtMTEtMTQgIEplZmYgTGF3ICA8bGF3QHJlZGhh dC5jb20+CisKKwkqIGdjYy5jLXRvcnR1cmUvY29tcGlsZS8yMDE4MTExNC5j OiBOZXcgdGVzdC4KKwogMjAxOC0xMS0xNCAgUmljaGFyZCBCaWVuZXIgIDxy Z3VlbnRoZXJAc3VzZS5kZT4KIAogCVBSIG1pZGRsZS1lbmQvODc5ODUKZGlm ZiAtLWdpdCBhL2djYy90ZXN0c3VpdGUvZ2NjLmMtdG9ydHVyZS9jb21waWxl LzIwMTgxMTE0LTEuYyBiL2djYy90ZXN0c3VpdGUvZ2NjLmMtdG9ydHVyZS9j b21waWxlLzIwMTgxMTE0LTEuYwpuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRl eCAwMDAwMDAwMDAwMC4uOWJjYzM5OTJmNjQKLS0tIC9kZXYvbnVsbAorKysg Yi9nY2MvdGVzdHN1aXRlL2djYy5jLXRvcnR1cmUvY29tcGlsZS8yMDE4MTEx NC0xLmMKQEAgLTAsMCArMSw2IEBACitpbnQKK192ZnByaW50Zl9yIChkb3Vi bGUgZnApCit7CisgIGlmIChfX2J1aWx0aW5fc2lnbmJpdCAoZnApKQorICAg IHJldHVybiAnLSc7Cit9Cg== --------------0EAAADF0D6F93CB29E8B6C20--