[PATCH] D33675: [DAG] Do MergeConsecutiveStores again before Instruction Selection

[Evgeny Astigeevich via llvm-commits]

Hi Eli,

Thank you for the clue. We will check if this is DAGCombiner::MergeStoresOfConstantsOrVecElts.

Thanks,
Evgeny

From: "Friedman, Eli" <efriedma at codeaurora.org>
Date: Wednesday, 29 November 2017 at 20:07
To: Nirav Davé <niravd at google.com>, "reviews+D33675+public+bfdf04a775858364 at reviews.llvm.org" <reviews+D33675+public+bfdf04a775858364 at reviews.llvm.org>
Cc: James Y Knight <jyknight at google.com>, "hfinkel at anl.gov" <hfinkel at anl.gov>, "llvm-dev at redking.me.uk" <llvm-dev at redking.me.uk>, Reid Kleckner <rnk at google.com>, James Molloy <James.Molloy at arm.com>, Evgeny Astigeevich <Evgeny.Astigeevich at arm.com>, "simon.dardis at mips.com" <simon.dardis at mips.com>, Nicolai Hähnle-Montoro <nhaehnle at gmail.com>, Javed Absar <Javed.Absar at arm.com>, llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: [PATCH] D33675: [DAG] Do MergeConsecutiveStores again before Instruction Selection

The example looks like there's a bug in the handling of truncstores: DAGCombiner::MergeStoresOfConstantsOrVecElts does "StoreInt |= C->getAPIntValue().zextOrTrunc(SizeInBits);", which doesn't correctly mask away the extra bits of a truncstore.  Not sure how to write a testcase, though.

-Eli

On 11/29/2017 11:01 AM, Nirav Davé wrote:
Disabled in ARM in r319331.

On Wed, Nov 29, 2017 at 12:31 PM, Nirav Davé <niravd at google.com<mailto:niravd at google.com>> wrote:
Sure If no one objects I'll disable this for just the ARM target.

-Nirav

On Wed, Nov 29, 2017 at 12:23 PM, Evgeny Astigeevich via Phabricator <reviews at reviews.llvm.org<mailto:reviews at reviews.llvm.org>> wrote:
eastig added a comment.

Hi Nirav,

Could you please revert the changes? They affected Arm targets (Thumb2 code).
The following sequence of stores:

  MOVS     r0,#0xe5
  STRB     r0,[r6,#0x1e5]
  MOVS     r0,#0xe4
  STRB     r0,[r6,#0x1e4]
  MOVS     r0,#0xe6
  STRB     r0,[r6,#0x1e6]
  MOVS     r0,#0xe7
  STRB     r0,[r6,#0x1e7]

is optimised into

  MVN      r0,#0x1b
  STR      r0,[r6,#0x1e4]

causing incorrect data to be written.

We are working on a reproducer.

Thanks,
Evgeny Astigeevich
The Arm Compiler Optimisation team


Repository:
  rL LLVM

https://reviews.llvm.org/D33675







--

Employee of Qualcomm Innovation Center, Inc.

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171129/e6b20fb6/attachment-0001.html>