[RFC PATCH v1 37/43] target/hexagon: Make HVX vector args. restrict *

Anton Johansson via posted 43 patches 1 year, 2 months ago
[RFC PATCH v1 37/43] target/hexagon: Make HVX vector args. restrict *
Posted by Anton Johansson via 1 year, 2 months ago
If pointer arguments to HVX helper functions are not marked restrict *,
then LLVM will assume that input vectors may alias and emit runtime
checks.

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 target/hexagon/mmvec/macros.h | 36 +++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h
index 1ceb9453ee..dfaefc6b26 100644
--- a/target/hexagon/mmvec/macros.h
+++ b/target/hexagon/mmvec/macros.h
@@ -23,26 +23,26 @@
 #include "mmvec/system_ext_mmvec.h"
 
 #ifndef QEMU_GENERATE
-#define VdV      (*(MMVector *)(VdV_void))
-#define VsV      (*(MMVector *)(VsV_void))
-#define VuV      (*(MMVector *)(VuV_void))
-#define VvV      (*(MMVector *)(VvV_void))
-#define VwV      (*(MMVector *)(VwV_void))
-#define VxV      (*(MMVector *)(VxV_void))
-#define VyV      (*(MMVector *)(VyV_void))
+#define VdV      (*(MMVector * restrict)(VdV_void))
+#define VsV      (*(MMVector * restrict)(VsV_void))
+#define VuV      (*(MMVector * restrict)(VuV_void))
+#define VvV      (*(MMVector * restrict)(VvV_void))
+#define VwV      (*(MMVector * restrict)(VwV_void))
+#define VxV      (*(MMVector * restrict)(VxV_void))
+#define VyV      (*(MMVector * restrict)(VyV_void))
 
-#define VddV     (*(MMVectorPair *)(VddV_void))
-#define VuuV     (*(MMVectorPair *)(VuuV_void))
-#define VvvV     (*(MMVectorPair *)(VvvV_void))
-#define VxxV     (*(MMVectorPair *)(VxxV_void))
+#define VddV     (*(MMVectorPair * restrict)(VddV_void))
+#define VuuV     (*(MMVectorPair * restrict)(VuuV_void))
+#define VvvV     (*(MMVectorPair * restrict)(VvvV_void))
+#define VxxV     (*(MMVectorPair * restrict)(VxxV_void))
 
-#define QeV      (*(MMQReg *)(QeV_void))
-#define QdV      (*(MMQReg *)(QdV_void))
-#define QsV      (*(MMQReg *)(QsV_void))
-#define QtV      (*(MMQReg *)(QtV_void))
-#define QuV      (*(MMQReg *)(QuV_void))
-#define QvV      (*(MMQReg *)(QvV_void))
-#define QxV      (*(MMQReg *)(QxV_void))
+#define QeV      (*(MMQReg * restrict)(QeV_void))
+#define QdV      (*(MMQReg * restrict)(QdV_void))
+#define QsV      (*(MMQReg * restrict)(QsV_void))
+#define QtV      (*(MMQReg * restrict)(QtV_void))
+#define QuV      (*(MMQReg * restrict)(QuV_void))
+#define QvV      (*(MMQReg * restrict)(QvV_void))
+#define QxV      (*(MMQReg * restrict)(QxV_void))
 #endif
 
 #define LOG_VTCM_BYTE(VA, MASK, VAL, IDX) \
-- 
2.45.2
Re: [RFC PATCH v1 37/43] target/hexagon: Make HVX vector args. restrict *
Posted by Philippe Mathieu-Daudé 1 year, 2 months ago
On 21/11/24 02:49, Anton Johansson wrote:
> If pointer arguments to HVX helper functions are not marked restrict *,
> then LLVM will assume that input vectors may alias and emit runtime
> checks.
> 
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   target/hexagon/mmvec/macros.h | 36 +++++++++++++++++------------------
>   1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h
> index 1ceb9453ee..dfaefc6b26 100644
> --- a/target/hexagon/mmvec/macros.h
> +++ b/target/hexagon/mmvec/macros.h
> @@ -23,26 +23,26 @@
>   #include "mmvec/system_ext_mmvec.h"
>   
>   #ifndef QEMU_GENERATE
> -#define VdV      (*(MMVector *)(VdV_void))
> -#define VsV      (*(MMVector *)(VsV_void))
> -#define VuV      (*(MMVector *)(VuV_void))
> -#define VvV      (*(MMVector *)(VvV_void))
> -#define VwV      (*(MMVector *)(VwV_void))
> -#define VxV      (*(MMVector *)(VxV_void))
> -#define VyV      (*(MMVector *)(VyV_void))
> +#define VdV      (*(MMVector * restrict)(VdV_void))
> +#define VsV      (*(MMVector * restrict)(VsV_void))
> +#define VuV      (*(MMVector * restrict)(VuV_void))
> +#define VvV      (*(MMVector * restrict)(VvV_void))
> +#define VwV      (*(MMVector * restrict)(VwV_void))
> +#define VxV      (*(MMVector * restrict)(VxV_void))
> +#define VyV      (*(MMVector * restrict)(VyV_void))
>   
> -#define VddV     (*(MMVectorPair *)(VddV_void))
> -#define VuuV     (*(MMVectorPair *)(VuuV_void))
> -#define VvvV     (*(MMVectorPair *)(VvvV_void))
> -#define VxxV     (*(MMVectorPair *)(VxxV_void))
> +#define VddV     (*(MMVectorPair * restrict)(VddV_void))
> +#define VuuV     (*(MMVectorPair * restrict)(VuuV_void))
> +#define VvvV     (*(MMVectorPair * restrict)(VvvV_void))
> +#define VxxV     (*(MMVectorPair * restrict)(VxxV_void))
>   
> -#define QeV      (*(MMQReg *)(QeV_void))
> -#define QdV      (*(MMQReg *)(QdV_void))
> -#define QsV      (*(MMQReg *)(QsV_void))
> -#define QtV      (*(MMQReg *)(QtV_void))
> -#define QuV      (*(MMQReg *)(QuV_void))
> -#define QvV      (*(MMQReg *)(QvV_void))
> -#define QxV      (*(MMQReg *)(QxV_void))
> +#define QeV      (*(MMQReg * restrict)(QeV_void))
> +#define QdV      (*(MMQReg * restrict)(QdV_void))
> +#define QsV      (*(MMQReg * restrict)(QsV_void))
> +#define QtV      (*(MMQReg * restrict)(QtV_void))
> +#define QuV      (*(MMQReg * restrict)(QuV_void))
> +#define QvV      (*(MMQReg * restrict)(QvV_void))
> +#define QxV      (*(MMQReg * restrict)(QxV_void))
>   #endif

Maybe we need to fix scripts/checkpatch.pl along?

ERROR: "foo * bar" should be "foo *bar"
#31: FILE: target/hexagon/mmvec/macros.h:26:
+#define VdV      (*(MMVector * restrict)(VdV_void))

ERROR: "foo * bar" should be "foo *bar"
#32: FILE: target/hexagon/mmvec/macros.h:27:
+#define VsV      (*(MMVector * restrict)(VsV_void))

ERROR: "foo * bar" should be "foo *bar"
#33: FILE: target/hexagon/mmvec/macros.h:28:
+#define VuV      (*(MMVector * restrict)(VuV_void))

[...]
Re: [RFC PATCH v1 37/43] target/hexagon: Make HVX vector args. restrict *
Posted by Paolo Bonzini 1 year, 2 months ago
On 11/25/24 12:36, Philippe Mathieu-Daudé wrote:
>> +#define QeV      (*(MMQReg * restrict)(QeV_void))
>> +#define QdV      (*(MMQReg * restrict)(QdV_void))
>> +#define QsV      (*(MMQReg * restrict)(QsV_void))
>> +#define QtV      (*(MMQReg * restrict)(QtV_void))
>> +#define QuV      (*(MMQReg * restrict)(QuV_void))
>> +#define QvV      (*(MMQReg * restrict)(QvV_void))
>> +#define QxV      (*(MMQReg * restrict)(QxV_void))
>>   #endif
> 
> Maybe we need to fix scripts/checkpatch.pl along?
> 
> ERROR: "foo * bar" should be "foo *bar"
> #31: FILE: target/hexagon/mmvec/macros.h:26:
> +#define VdV      (*(MMVector * restrict)(VdV_void))
> 
> ERROR: "foo * bar" should be "foo *bar"
> #32: FILE: target/hexagon/mmvec/macros.h:27:
> +#define VsV      (*(MMVector * restrict)(VsV_void))
> 
> ERROR: "foo * bar" should be "foo *bar"
> #33: FILE: target/hexagon/mmvec/macros.h:28:
> +#define VuV      (*(MMVector * restrict)(VuV_void))

I think checkpatch.pl has a point here. :)

Paolo


Re: [RFC PATCH v1 37/43] target/hexagon: Make HVX vector args. restrict *
Posted by Anton Johansson via 1 year, 2 months ago
On 25/11/24, Paolo Bonzini wrote:
> On 11/25/24 12:36, Philippe Mathieu-Daudé wrote:
> > > +#define QeV      (*(MMQReg * restrict)(QeV_void))
> > > +#define QdV      (*(MMQReg * restrict)(QdV_void))
> > > +#define QsV      (*(MMQReg * restrict)(QsV_void))
> > > +#define QtV      (*(MMQReg * restrict)(QtV_void))
> > > +#define QuV      (*(MMQReg * restrict)(QuV_void))
> > > +#define QvV      (*(MMQReg * restrict)(QvV_void))
> > > +#define QxV      (*(MMQReg * restrict)(QxV_void))
> > >   #endif
> > 
> > Maybe we need to fix scripts/checkpatch.pl along?
> > 
> > ERROR: "foo * bar" should be "foo *bar"
> > #31: FILE: target/hexagon/mmvec/macros.h:26:
> > +#define VdV      (*(MMVector * restrict)(VdV_void))
> > 
> > ERROR: "foo * bar" should be "foo *bar"
> > #32: FILE: target/hexagon/mmvec/macros.h:27:
> > +#define VsV      (*(MMVector * restrict)(VsV_void))
> > 
> > ERROR: "foo * bar" should be "foo *bar"
> > #33: FILE: target/hexagon/mmvec/macros.h:28:
> > +#define VuV      (*(MMVector * restrict)(VuV_void))
> 
> I think checkpatch.pl has a point here. :)

I'll switch to `*restrict`!:)

//Anton

Re: [RFC PATCH v1 37/43] target/hexagon: Make HVX vector args. restrict *
Posted by Brian Cain 1 year, 2 months ago
On 12/3/2024 12:57 PM, Anton Johansson via wrote:
> On 25/11/24, Paolo Bonzini wrote:
>> On 11/25/24 12:36, Philippe Mathieu-Daudé wrote:
>>>> +#define QeV      (*(MMQReg * restrict)(QeV_void))
>>>> +#define QdV      (*(MMQReg * restrict)(QdV_void))
>>>> +#define QsV      (*(MMQReg * restrict)(QsV_void))
>>>> +#define QtV      (*(MMQReg * restrict)(QtV_void))
>>>> +#define QuV      (*(MMQReg * restrict)(QuV_void))
>>>> +#define QvV      (*(MMQReg * restrict)(QvV_void))
>>>> +#define QxV      (*(MMQReg * restrict)(QxV_void))
>>>>    #endif
>>> Maybe we need to fix scripts/checkpatch.pl along?
>>>
>>> ERROR: "foo * bar" should be "foo *bar"
>>> #31: FILE: target/hexagon/mmvec/macros.h:26:
>>> +#define VdV      (*(MMVector * restrict)(VdV_void))
>>>
>>> ERROR: "foo * bar" should be "foo *bar"
>>> #32: FILE: target/hexagon/mmvec/macros.h:27:
>>> +#define VsV      (*(MMVector * restrict)(VsV_void))
>>>
>>> ERROR: "foo * bar" should be "foo *bar"
>>> #33: FILE: target/hexagon/mmvec/macros.h:28:
>>> +#define VuV      (*(MMVector * restrict)(VuV_void))
>> I think checkpatch.pl has a point here. :)
> I'll switch to `*restrict`!:)


With this change to fix checkpatch,

Reviewed-by: Brian Cain <brian.cain@oss.qualcomm.com>


>
> //Anton
>