* [PATCH] [v2] dma-mapping: work around clang bug
@ 2019-03-07 8:52 ` Arnd Bergmann
0 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2019-03-07 8:52 UTC (permalink / raw)
To: Christoph Hellwig, Marek Szyprowski
Cc: Nick Desaulniers, Arnd Bergmann, Robin Murphy,
Jesper Dangaard Brouer, Paul Burton, Geert Uytterhoeven, iommu,
linux-kernel
Clang has a rather annoying behavior of checking for integer
arithmetic problems in code paths that are discarded by gcc
before that perfoms the same checks.
For DMA_BIT_MASK(64), this leads to a warning despite the
result of the macro being completely sensible:
arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
.coherent_dma_mask = DMA_BIT_MASK(64),
The best workaround I could come up with is to shift the
value twice, which makes the macro way less readable but
always has the same result.
Link: https://bugs.llvm.org/show_bug.cgi?id=38789
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: fix off-by-one error
---
include/linux/dma-mapping.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 75e60be91e5f..9e438fe6b130 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -138,7 +138,8 @@ struct dma_map_ops {
extern const struct dma_map_ops dma_virt_ops;
extern const struct dma_map_ops dma_dummy_ops;
-#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
+/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
+#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
#define DMA_MASK_NONE 0x0ULL
--
2.20.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] [v2] dma-mapping: work around clang bug
@ 2019-03-07 8:52 ` Arnd Bergmann
0 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2019-03-07 8:52 UTC (permalink / raw)
To: Christoph Hellwig, Marek Szyprowski
Cc: Arnd Bergmann, Nick Desaulniers,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Paul Burton,
Geert Uytterhoeven, Jesper Dangaard Brouer, Robin Murphy
Clang has a rather annoying behavior of checking for integer
arithmetic problems in code paths that are discarded by gcc
before that perfoms the same checks.
For DMA_BIT_MASK(64), this leads to a warning despite the
result of the macro being completely sensible:
arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
.coherent_dma_mask = DMA_BIT_MASK(64),
The best workaround I could come up with is to shift the
value twice, which makes the macro way less readable but
always has the same result.
Link: https://bugs.llvm.org/show_bug.cgi?id=38789
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
v2: fix off-by-one error
---
include/linux/dma-mapping.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 75e60be91e5f..9e438fe6b130 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -138,7 +138,8 @@ struct dma_map_ops {
extern const struct dma_map_ops dma_virt_ops;
extern const struct dma_map_ops dma_dummy_ops;
-#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
+/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
+#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
#define DMA_MASK_NONE 0x0ULL
--
2.20.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] [v2] dma-mapping: work around clang bug
@ 2019-03-07 9:17 ` Robin Murphy
0 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2019-03-07 9:17 UTC (permalink / raw)
To: Arnd Bergmann, Christoph Hellwig, Marek Szyprowski
Cc: Nick Desaulniers, Jesper Dangaard Brouer, Paul Burton,
Geert Uytterhoeven, iommu, linux-kernel
Hi Arnd,
On 2019-03-07 8:52 am, Arnd Bergmann wrote:
> Clang has a rather annoying behavior of checking for integer
> arithmetic problems in code paths that are discarded by gcc
> before that perfoms the same checks.
>
> For DMA_BIT_MASK(64), this leads to a warning despite the
> result of the macro being completely sensible:
>
> arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
> .coherent_dma_mask = DMA_BIT_MASK(64),
>
> The best workaround I could come up with is to shift the
> value twice, which makes the macro way less readable but
> always has the same result.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=38789
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: fix off-by-one error
> ---
> include/linux/dma-mapping.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 75e60be91e5f..9e438fe6b130 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -138,7 +138,8 @@ struct dma_map_ops {
> extern const struct dma_map_ops dma_virt_ops;
> extern const struct dma_map_ops dma_dummy_ops;
>
> -#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
> +#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter
in most cases, but it could potentially happen at runtime where callers
use a non-constant argument. However, it also means we don't need to
special-case 64 any more (since that's there to avoid the same thing
anyway), so we could simply flip that to handle 0 instead.
FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)",
but that may not be to everyone's taste.
Robin.
>
> #define DMA_MASK_NONE 0x0ULL
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v2] dma-mapping: work around clang bug
@ 2019-03-07 9:17 ` Robin Murphy
0 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2019-03-07 9:17 UTC (permalink / raw)
To: Arnd Bergmann, Christoph Hellwig, Marek Szyprowski
Cc: Nick Desaulniers,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Paul Burton,
Geert Uytterhoeven, Jesper Dangaard Brouer
Hi Arnd,
On 2019-03-07 8:52 am, Arnd Bergmann wrote:
> Clang has a rather annoying behavior of checking for integer
> arithmetic problems in code paths that are discarded by gcc
> before that perfoms the same checks.
>
> For DMA_BIT_MASK(64), this leads to a warning despite the
> result of the macro being completely sensible:
>
> arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
> .coherent_dma_mask = DMA_BIT_MASK(64),
>
> The best workaround I could come up with is to shift the
> value twice, which makes the macro way less readable but
> always has the same result.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=38789
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
> v2: fix off-by-one error
> ---
> include/linux/dma-mapping.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 75e60be91e5f..9e438fe6b130 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -138,7 +138,8 @@ struct dma_map_ops {
> extern const struct dma_map_ops dma_virt_ops;
> extern const struct dma_map_ops dma_dummy_ops;
>
> -#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
> +#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter
in most cases, but it could potentially happen at runtime where callers
use a non-constant argument. However, it also means we don't need to
special-case 64 any more (since that's there to avoid the same thing
anyway), so we could simply flip that to handle 0 instead.
FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)",
but that may not be to everyone's taste.
Robin.
>
> #define DMA_MASK_NONE 0x0ULL
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v2] dma-mapping: work around clang bug
2019-03-07 9:17 ` Robin Murphy
(?)
@ 2019-03-07 9:28 ` Arnd Bergmann
2019-03-07 9:34 ` Geert Uytterhoeven
2019-03-07 9:43 ` Robin Murphy
-1 siblings, 2 replies; 8+ messages in thread
From: Arnd Bergmann @ 2019-03-07 9:28 UTC (permalink / raw)
To: Robin Murphy
Cc: Christoph Hellwig, Marek Szyprowski, Nick Desaulniers,
Jesper Dangaard Brouer, Paul Burton, Geert Uytterhoeven,
open list:IOMMU DRIVERS, Linux Kernel Mailing List
On Thu, Mar 7, 2019 at 10:17 AM Robin Murphy <robin.murphy@arm.com> wrote:
> On 2019-03-07 8:52 am, Arnd Bergmann wrote:
> >
> > -#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> > +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
> > +#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
>
> I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter
> in most cases, but it could potentially happen at runtime where callers
> use a non-constant argument. However, it also means we don't need to
> special-case 64 any more (since that's there to avoid the same thing
> anyway), so we could simply flip that to handle 0 instead.
Yes, good idea.
> FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)",
> but that may not be to everyone's taste.
I like that. So shall we do this?
/*
* Shifting '2' instead of '1' because of
* https://bugs.llvm.org/show_bug.cgi?id=38789
*/
#define DMA_BIT_MASK(n) (((n) == 0) ? 0ULL : ((2ULL<<((n)-1)))-1)
Arnd
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v2] dma-mapping: work around clang bug
2019-03-07 9:28 ` Arnd Bergmann
@ 2019-03-07 9:34 ` Geert Uytterhoeven
2019-03-07 9:43 ` Robin Murphy
1 sibling, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2019-03-07 9:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Robin Murphy, Christoph Hellwig, Marek Szyprowski,
Nick Desaulniers, Jesper Dangaard Brouer, Paul Burton,
open list:IOMMU DRIVERS, Linux Kernel Mailing List
On Thu, Mar 7, 2019 at 10:28 AM Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Mar 7, 2019 at 10:17 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > On 2019-03-07 8:52 am, Arnd Bergmann wrote:
> > >
> > > -#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> > > +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
> > > +#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
> >
> > I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter
> > in most cases, but it could potentially happen at runtime where callers
> > use a non-constant argument. However, it also means we don't need to
> > special-case 64 any more (since that's there to avoid the same thing
> > anyway), so we could simply flip that to handle 0 instead.
>
> Yes, good idea.
>
> > FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)",
> > but that may not be to everyone's taste.
>
> I like that. So shall we do this?
>
> /*
> * Shifting '2' instead of '1' because of
> * https://bugs.llvm.org/show_bug.cgi?id=38789
> */
> #define DMA_BIT_MASK(n) (((n) == 0) ? 0ULL : ((2ULL<<((n)-1)))-1)
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v2] dma-mapping: work around clang bug
@ 2019-03-07 9:43 ` Robin Murphy
0 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2019-03-07 9:43 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Christoph Hellwig, Marek Szyprowski, Nick Desaulniers,
Jesper Dangaard Brouer, Paul Burton, Geert Uytterhoeven,
open list:IOMMU DRIVERS, Linux Kernel Mailing List
On 2019-03-07 9:28 am, Arnd Bergmann wrote:
> On Thu, Mar 7, 2019 at 10:17 AM Robin Murphy <robin.murphy@arm.com> wrote:
>> On 2019-03-07 8:52 am, Arnd Bergmann wrote:
>>>
>>> -#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
>>> +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
>>> +#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
>>
>> I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter
>> in most cases, but it could potentially happen at runtime where callers
>> use a non-constant argument. However, it also means we don't need to
>> special-case 64 any more (since that's there to avoid the same thing
>> anyway), so we could simply flip that to handle 0 instead.
>
> Yes, good idea.
>
>> FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)",
>> but that may not be to everyone's taste.
>
> I like that. So shall we do this?
>
> /*
> * Shifting '2' instead of '1' because of
> * https://bugs.llvm.org/show_bug.cgi?id=38789
> */
> #define DMA_BIT_MASK(n) (((n) == 0) ? 0ULL : ((2ULL<<((n)-1)))-1)
Neat - it was too early in the morning for me to think of a succinct way
to comment it, but that's great. I suspect there may be a redundant set
of parentheses around the shift still, but other than that,
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Cheers,
Robin.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v2] dma-mapping: work around clang bug
@ 2019-03-07 9:43 ` Robin Murphy
0 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2019-03-07 9:43 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Nick Desaulniers, open list:IOMMU DRIVERS,
Linux Kernel Mailing List, Paul Burton, Geert Uytterhoeven,
Jesper Dangaard Brouer, Christoph Hellwig
On 2019-03-07 9:28 am, Arnd Bergmann wrote:
> On Thu, Mar 7, 2019 at 10:17 AM Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 2019-03-07 8:52 am, Arnd Bergmann wrote:
>>>
>>> -#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
>>> +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
>>> +#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
>>
>> I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter
>> in most cases, but it could potentially happen at runtime where callers
>> use a non-constant argument. However, it also means we don't need to
>> special-case 64 any more (since that's there to avoid the same thing
>> anyway), so we could simply flip that to handle 0 instead.
>
> Yes, good idea.
>
>> FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)",
>> but that may not be to everyone's taste.
>
> I like that. So shall we do this?
>
> /*
> * Shifting '2' instead of '1' because of
> * https://bugs.llvm.org/show_bug.cgi?id=38789
> */
> #define DMA_BIT_MASK(n) (((n) == 0) ? 0ULL : ((2ULL<<((n)-1)))-1)
Neat - it was too early in the morning for me to think of a succinct way
to comment it, but that's great. I suspect there may be a redundant set
of parentheses around the shift still, but other than that,
Reviewed-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
Cheers,
Robin.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-07 9:43 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07 8:52 [PATCH] [v2] dma-mapping: work around clang bug Arnd Bergmann
2019-03-07 8:52 ` Arnd Bergmann
2019-03-07 9:17 ` Robin Murphy
2019-03-07 9:17 ` Robin Murphy
2019-03-07 9:28 ` Arnd Bergmann
2019-03-07 9:34 ` Geert Uytterhoeven
2019-03-07 9:43 ` Robin Murphy
2019-03-07 9:43 ` Robin Murphy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.