Hi Jeevitha,
Please find my comments inline
On 27/11/25 10:25 am, jeevitha wrote:
Gentle Ping!
Please review.
Thanks and Regards,
Jeevitha P.
On 20/11/25 9:52 am, jeevitha wrote:
Hi All,
The following patch is to fix PR122665 has been bootstrapped and regtested on
powerpc64le-linux. Is it OK for trunk?
The existing smul<mode>3_highpart and umul<mode>3_highpart patterns
incorrectly defined the high-part multiply by shifting both operands
before multiplication. This does not match the semantics of the
instructions vmulhs<wd> and vmulhu<wd>, which perform a widened
multiplication and then return the high part.
This patch replaces the incorrect patterns with UNSPEC_VMULHS and
UNSPEC_VMULHU forms, and updates the predicate to use
altivec_register_operand, since these instructions accept only
Altivec registers.
2025-11-19 Jeevitha Palanisamy <[email protected]>
gcc/
PR target/122665
* config/rs6000/vsx.md (UNSPEC_VMULHS, UNSPEC_VMULHU): New.
(smul<mode>3_highpart): Use UNSPEC_VMULHS.
(umul<mode>3_highpart): Use UNSPEC_VMULHU.
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index dd3573b8086..e66462f1c4b 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -362,6 +362,8 @@
UNSPEC_REPLACE_UN
UNSPEC_VDIVES
UNSPEC_VDIVEU
+ UNSPEC_VMULHS
+ UNSPEC_VMULHU
UNSPEC_VMSUMCUD
UNSPEC_XXEVAL
UNSPEC_XXSPLTIW
@@ -6546,25 +6548,19 @@
(set_attr "size" "<bits>")])
Introduces UNSPEC_VMULHS and UNSPEC_VMULHU to explicitly represent these
special operations without implying incorrect semantics.
This looks good to me
(define_insn "smul<mode>3_highpart"
- [(set (match_operand:VIlong 0 "vsx_register_operand" "=v")
- (mult:VIlong (ashiftrt
- (match_operand:VIlong 1 "vsx_register_operand" "v")
- (const_int 32))
- (ashiftrt
- (match_operand:VIlong 2 "vsx_register_operand" "v")
- (const_int 32))))]
+ [(set (match_operand:VIlong 0 "altivec_register_operand" "=v")
+ (unspec:VIlong [(match_operand:VIlong 1 "altivec_register_operand" "v")
+ (match_operand:VIlong 2 "altivec_register_operand" "v")]
+ UNSPEC_VMULHS))]
"TARGET_POWER10"
"vmulhs<wd> %0,%1,%2"
[(set_attr "type" "veccomplex")])
(define_insn "umul<mode>3_highpart"
- [(set (match_operand:VIlong 0 "vsx_register_operand" "=v")
- (us_mult:VIlong (ashiftrt
- (match_operand:VIlong 1 "vsx_register_operand" "v")
- (const_int 32))
- (ashiftrt
- (match_operand:VIlong 2 "vsx_register_operand" "v")
- (const_int 32))))]
+ [(set (match_operand:VIlong 0 "altivec_register_operand" "=v")
+ (unspec:VIlong [(match_operand:VIlong 1 "altivec_register_operand"
"v")
+ (match_operand:VIlong 2 "altivec_register_operand"
"v")]
+ UNSPEC_VMULHU))]
"TARGET_POWER10"
"vmulhu<wd> %0,%1,%2"
[(set_attr "type" "veccomplex")])
Changed from vsx_register_operand to altivec_register_operand since
these instructions only accept Altivec registers (more restrictive).
Both smul<mode>3_highpart and umul<mode>3_highpart now use UNSPEC forms
instead of the shift-then-multiply pattern.
The changes look good to me but, don't you think it would be a good idea
to move the above changes to altivec.md file?
Regards,
Manjunath S Matti.
