Hi Dennis,
On 6/27/19 4:58 PM, Dennis Zhang wrote:
Hi Kyrill,
Thanks for the review!
On 6/24/19 5:27 PM, Kyrill Tkachov wrote:
Hi Dennis,
On 6/24/19 4:13 PM, Dennis Zhang wrote:
Hi,
A number of Arm define_expand patterns have specified constraints for
their operands. But the constraint strings are ignored at expand time
and are therefore redundant/useless. We now avoid specifying constraints
in new define_expands, but we should clean up the existing define_expand
definitions.
For example, the constraint "=r" is removed in the following case:
(define_expand "reload_inhi"
[(parallel [(match_operand:HI 0 "s_register_operand" "=r")
The "" marks with an empty constraint in define_expand are removed as
well.
The patch is tested with the build configuration of
--target=arm-linux-gnueabi and it passes gcc/testsuite.
Thank you for the patch.
Unfortunately I've hit an ICE building an arm-none-eabi target with your
patch.
This appears to be due to:
@@ -6767,9 +6767,9 @@
;; temporary if the address isn't offsettable -- push_reload doesn't seem
;; to take any notice of the "o" constraints on reload_memory_operand
operand.
(define_expand "reload_outhi"
- [(parallel [(match_operand:HI 0 "arm_reload_memory_operand" "=o")
- (match_operand:HI 1 "s_register_operand" "r")
- (match_operand:DI 2 "s_register_operand" "=&l")])]
+ [(parallel [(match_operand:HI 0 "arm_reload_memory_operand")
+ (match_operand:HI 1 "s_register_operand")
+ (match_operand:DI 2 "s_register_operand")])]
"TARGET_EITHER"
"if (TARGET_ARM)
arm_reload_out_hi (operands);
@@ -6780,9 +6780,9 @@
)
(define_expand "reload_inhi"
- [(parallel [(match_operand:HI 0 "s_register_operand" "=r")
- (match_operand:HI 1 "arm_reload_memory_operand" "o")
- (match_operand:DI 2 "s_register_operand" "=&r")])]
+ [(parallel [(match_operand:HI 0 "s_register_operand")
+ (match_operand:HI 1 "arm_reload_memory_operand")
+ (match_operand:DI 2 "s_register_operand")])]
"TARGET_EITHER"
"
if (TARGET_ARM)
the reload_in and reload_out patterns are somewhat special:
https://gcc.gnu.org/onlinedocs/gccint/Standard-Names.html#Standard-Names
where the constraints seems to matter.
We should migrate these patterns to the recommended secondary_reload
hook, but that would be a separate patches.
For now, please try removing changes to these patterns and making sure
that the build succeeds.
Thanks,
Kyrill
The special exception, reload_inm and reload_outm patterns are fixed.
Their constraints are left untouched as original in order to work
correctly in default_secondary_reload, gcc/targhook.c.
The updated patch is tested for targets: arm-none-linux-gnueabi,
arm-none-linux-gnueabihf, arm-linux-gnueabi, and arm-linux-gnueabihf.
And it survives in testsuite regression.
Thanks, I've committed this on your behalf with r272779.
Kyrill
gcc/ChangeLog:
2019-06-21 Dennis Zhang <dennis.zh...@arm.com>
* config/arm/arm.md: Remove redundant constraints from
define_expand but leave reload_inm and reload_outm patterns
untouched since they need special constraints to work.
* config/arm/arm-fixed.md: Remove redundant constraints from
define_expand.
* config/arm/iwmmxt.md: Likewise.
* config/arm/neon.md: Likewise.
* config/arm/sync.md: Likewise.
* config/arm/thumb1.md: Likewise.
* config/arm/vec-common.md: Likewise.
