Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness

On Wednesday, October 19, 2016 4:01:58 PM CEST Ard Biesheuvel wrote:

On 19 October 2016 at 15:59, Ard Biesheuvel [off-list ref] wrote:

quoted

On 19 October 2016 at 14:35, Will Deacon [off-list ref] wrote:

quoted

On Mon, Oct 17, 2016 at 08:43:19PM +0100, Ard Biesheuvel wrote:

quoted

On 17 October 2016 at 19:38, Will Deacon [off-list ref] wrote:

Yes, and that would be perfectly legal from a correctness point of
view, and would likely help performance as well. By using
__builtin_constant_p(), you are choosing to perform a build time
evaluation of an expression that would ordinarily be evaluated only at
runtime. This implies that you have to address undefined behavior at
build time rather than at runtime as well.

quoted

quoted

If order_base_2() is not defined for input 0, it should BUG() in that
case, and the associated __builtin_unreachable() should prevent the
special version from being emitted. If order_base_2() is defined for input
0, it should not invoke ilog2() with that argument, and the problem should
go away as well.

I don't necessarily think it should BUG() if it's not defined for input
0; things like __ffs don't do that and we'd be introducing conditional
checks for cases that should not happen. The comment above order_base_2
does suggest that ob2(0) should return 0, but it can actually end up
invoking ilog2(-1), which is obviously wrong.

I could update the comment, but that doesn't fix the build issue.

Fixing roundup_pow_of_two() [which is arguably incorrect]

I just spotted the comment that says it is undefined. But that means
it could legally return 1 for input 0, i suppose

I think having the link error in roundup_pow_of_two() is safer than
returning 1.

Why not turn it into a runtime warning in this driver?

diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
index cecb0fdfaef6..711d1d9842cc 100644
--- a/drivers/clk/mvebu/armada-37xx-periph.c
+++ b/drivers/clk/mvebu/armada-37xx-periph.c

@@ -349,8 +349,10 @@ static int armada_3700_add_composite_clk(const struct clk_periph_data *data,
 			rate->reg = reg + (u64)rate->reg;
 			for (clkt = rate->table; clkt->div; clkt++)
 				table_size++;
-			rate->width = order_base_2(table_size);
-			rate->lock = lock;
+			if (!WARN_ON(table_size == 0)) {
+				rate->width = order_base_2(table_size);
+				rate->lock = lock;
+			}
 		}
 	}
 

	
	Arnd