)]}'
{
  "commit": "d0eff1793dc844c2a9532d8ef1a07ba137cfb4e6",
  "tree": "6c532cf3e87b664788638461caf942a03da5aadf",
  "parents": [
    "c6b81c451983f1b9a6e184f137819abf4978eddd"
  ],
  "author": {
    "name": "Jakub Jelinek",
    "email": "jakub@redhat.com",
    "time": "Wed Feb 26 09:33:48 2020 +0100"
  },
  "committer": {
    "name": "Jakub Jelinek",
    "email": "jakub@redhat.com",
    "time": "Wed Feb 26 09:35:52 2020 +0100"
  },
  "message": "store-merging: Fix coalesce_immediate_stores [PR93820]\n\nThe following testcase is miscompiled in 8+.\nThe problem is that check_no_overlap has a special case for INTEGER_CST\nmarked stores (i.e. stores of constants), if both all currenly merged stores\nand the one under consideration for merging with them are marked that way,\nit anticipates that other INTEGER_CST marked stores that overlap with those\nand precede those (have smaller info-\u003eorder) could be merged with those and\ndoesn\u0027t punt for them.\nIn PR86844 and PR87859 fixes I\u0027ve then added quite large code that is\nperformed after check_no_overlap and tries to find out if we need and can\nmerge further INTEGER_CST marked stores, or need to punt.\nUnfortunately, that code is there only in the overlapping case code and\nthe testcase below shows that we really need it even in the adjacent store\ncase.  After sort_by_bitpos we have:\nbitpos\twidth\torder\trhs_code\n96\t32\t3\tINTEGER_CST\n128\t32\t1\tINTEGER_CST\n128\t128\t2\tINTEGER_CST\n192\t32\t0\tMEM_REF\nBecause of the missing PR86844/PR87859-ish code in the adjacent store\ncase, we merge the adjacent (memory wise) stores 96/32/3 and 128/32/1,\nand then we consider the 128-bit store which is in program-order in between\nthem, but in this case we punt, because the merging would extend the\nmerged store region from bitpos 96 and 64-bits to bitpos 96 and 160-bits\nand that has an overlap with an incompatible store (the MEM_REF one).\nThe problem is that we can\u0027t really punt this way, because the 128-bit\nstore is in between those two we\u0027ve merged already, so either we manage\nto merge even that one together with the others, or would need to avoid\nalready merging the 96/32/3 and 128/32/1 stores together.\nNow, rather than copying around the PR86844/PR87859 code to the other spot,\nwe can actually just use the overlapping code, merge_overlapping is really\na superset of merge_into, so that is what the patch does.  If doing\nadjacent store merge for rhs_code other than INTEGER_CST, I believe the\ncurrent code is already fine, check_no_overlap in that case doesn\u0027t make\nthe exception and will punt if there is some earlier (smaller order)\nnon-mergeable overlapping store.  There is just one case that could be\nproblematic, if the merged_store has BIT_INSERT_EXPRs in them and the\nnew store is a constant store (INTEGER_CST rhs_code), then check_no_overlap\nwould do the exception and still would allow the special case.  But we\nreally shouldn\u0027t have the special case in that case, so this patch also\nchanges check_no_overlap to just have a bool whether we should have the\nspecial case or not.\n\nNote, as I said in the PR, for GCC11 we could consider performing some kind\nof cheap DSE during the store merging (perhaps guarded with flag_tree_dse).\nAnd another thing to consider is only consider as problematic non-mergeable\nstores that not only have order smaller than last_order as currently, but\nalso have order larger than first_order, as in this testcase if we actually\nignored (not merged with anything at all) the 192/32/0 store, because it is\nnot in between the other stores we\u0027d merge, it would be fine to merge the\nother 3 stores, though of course the testcase can be easily adjusted by\nputting the 192/32 store after the 128/32 store and then this patch would be\nstill needed.  Though, I think I\u0027d need more time thinking this over.\n\n2020-02-26  Jakub Jelinek  \u003cjakub@redhat.com\u003e\n\n\tPR tree-optimization/93820\n\t* gimple-ssa-store-merging.c (check_no_overlap): Change RHS_CODE\n\targument to ALL_INTEGER_CST_P boolean.\n\t(imm_store_chain_info::try_coalesce_bswap): Adjust caller.\n\t(imm_store_chain_info::coalesce_immediate_stores): Likewise.  Handle\n\tadjacent INTEGER_CST store into merged_store-\u003eonly_constants like\n\toverlapping one.\n\n\t* gcc.dg/pr93820.c: New test.\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "c8dd6d0c613c662475ced45bcc6c1adb2d700aa3",
      "old_mode": 33188,
      "old_path": "gcc/ChangeLog",
      "new_id": "cf37d814a66b2ae13e4a57486e2503842c408adc",
      "new_mode": 33188,
      "new_path": "gcc/ChangeLog"
    },
    {
      "type": "modify",
      "old_id": "4a8cf6f847fd21296df3fb548db4a55ee8871fef",
      "old_mode": 33188,
      "old_path": "gcc/gimple-ssa-store-merging.c",
      "new_id": "550f782813b85c14003bd12b510ff6c1a947e2f5",
      "new_mode": 33188,
      "new_path": "gcc/gimple-ssa-store-merging.c"
    },
    {
      "type": "modify",
      "old_id": "86bc08d4bb78540d88f5f242818b7315304af560",
      "old_mode": 33188,
      "old_path": "gcc/testsuite/ChangeLog",
      "new_id": "100489e1201c8b3922736a9393418176fd712f75",
      "new_mode": 33188,
      "new_path": "gcc/testsuite/ChangeLog"
    },
    {
      "type": "add",
      "old_id": "0000000000000000000000000000000000000000",
      "old_mode": 0,
      "old_path": "/dev/null",
      "new_id": "98d8c20a178f80525a3ca20732cfee89565e7ac9",
      "new_mode": 33188,
      "new_path": "gcc/testsuite/gcc.dg/pr93820.c"
    }
  ]
}
