Hi, > On Tue, 8 Oct 2013 22:50:21, Eric Botcazou wrote: >> >>> I agree, that assigning a non-BLKmode to structures with zero-sized arrays >>> should be considered a bug. >> >> Fine, then let's apply Martin's patch, on mainline at least. >> > > That would definitely be a good move. Maybe someone should approve it? > >> But this testcase is invalid on STRICT_ALIGNMENT platforms: xx is pointer to >> a >> type with 4-byte alignment so its value must be a multiple of 4. > > Then you probably win. But I still have some doubts. > > I had to use this silly alignment/pack(4) to circumvent this statement > in compute_record_mode: > > /* If structure's known alignment is less than what the scalar > mode would need, and it matters, then stick with BLKmode. */ > if (TYPE_MODE (type) != BLKmode > && STRICT_ALIGNMENT > && ! (TYPE_ALIGN (type)>= BIGGEST_ALIGNMENT > || TYPE_ALIGN (type)>= GET_MODE_ALIGNMENT (TYPE_MODE (type)))) > { > /* If this is the only reason this type is BLKmode, then > don't force containing types to be BLKmode. */ > TYPE_NO_FORCE_BLK (type) = 1; > SET_TYPE_MODE (type, BLKmode); > } > > But there are at least two targets where STRICT_ALIGNMENT = 0 > and SLOW_UNALIGNED_ACCESS != 0: rs6000 and alpha. > > This example with a byte-aligned structure will on one of these targets > likely execute this code path in expand_expr_real_1/case MEM_REF: > > else if (SLOW_UNALIGNED_ACCESS (mode, align)) > temp = extract_bit_field (temp, GET_MODE_BITSIZE (mode), > 0, TYPE_UNSIGNED (TREE_TYPE (exp)), > (modifier == EXPAND_STACK_PARM > ? NULL_RTX : target), > mode, mode); > > This looks wrong, but unfortunately I cannot test on these targets... >
Hmm, well, the condition that would be necessary to execute that code path would be STRICT_ALIGNMENT = 0 and SLOW_UNALIGNED_ACCESS != 0 for any integer mode. The only target that is close to hit this "bug" is rs6000: #define STRICT_ALIGNMENT 0 #define SLOW_UNALIGNED_ACCESS(MODE, ALIGN) \ (STRICT_ALIGNMENT \ || (((MODE) == SFmode || (MODE) == DFmode || (MODE) == TFmode \ || (MODE) == SDmode || (MODE) == DDmode || (MODE) == TDmode) \ && (ALIGN) < 32) \ || (VECTOR_MODE_P ((MODE)) && (((int)(ALIGN)) < VECTOR_ALIGN (MODE)))) but, luckily this is 0 for all integer modes. So I am now convinced, there won't be any valid example with unions that executes this code path. Therefore I updated Martin's previous patch, to meet Eric's request: That is to only handle zero-sized arrays at the end of the structure. Boot-strapped and regression-tested on x86_64-linux-gnu. Ok for trunk? Regards Bernd.
2013-10-22 Martin Jambor <mjam...@suse.cz> Bernd Edlinger <bernd.edlin...@hotmail.de> PR middle-end/57748 * stor-layout.c (compute_record_mode): Treat trailing zero-sized array fields like incomplete types. testsuite: 2013-10-22 Bernd Edlinger <bernd.edlin...@hotmail.de> PR middle-end/57748 * gcc.dg/torture/pr57748-3.c: New test.
patch-pr57748-2.diff
Description: Binary data