release/18.x: [Mips] Fix missing sign extension in expansion of sub-word atomic max (#77072) #84566
Conversation
…llvm#77072) Add sign extension "SEB/SEH" before compare. Fix llvm#61881 (cherry picked from commit 755b439)
|
@topperc What do you think about merging this PR to the release branch? |
| @@ -2001,8 +2225,6 @@ define i16 @test_umax_16(ptr nocapture %ptr, i16 signext %val) { | |||
| ; MIPSELR6-NEXT: $BB6_1: # %entry | |||
| ; MIPSELR6-NEXT: # =>This Inner Loop Header: Depth=1 | |||
| ; MIPSELR6-NEXT: ll $2, 0($6) | |||
| ; MIPSELR6-NEXT: and $2, $2, $8 | |||
There was a problem hiding this comment.
@brad0 I think I missed this in the previous review. Why is it ok to remove the AND from the unsigned tests? My original comment about the AND being unnecessary was for the signed cases.
There was a problem hiding this comment.
@brad0 I think I missed this in the previous review. Why is it ok to remove the AND from the unsigned tests? My original command about the AND being unnecssary was for the signed cases.
Looking at the PR I am not sure that was clear and @yingopq might have interpreted your comment as to remove all together.
There was a problem hiding this comment.
@topperc Sorry, I misunderstand the previous review opinion. I tested the code with removing all and and did not find any issue, then I submit the patch. If you think it was wrong, I would submit again. Is it need?
There was a problem hiding this comment.
@yingopq I'm not really a Mips expert so I'm not sure. But my understanding is that the and was clearing the upper and lower bits so that the unsigned min/max would only consider the byte or half word that it should and not any other values around it in memory.
There was a problem hiding this comment.
Yes, I would submit again soon. Thanks.
Backport 755b439
Requested by: @brad0