<div dir="ltr">Can you help a bit?<div><br></div><div>I try to work with DSE but I got the following assert:</div><div><div>opt: /home/xbolva00/LLVM/llvm/include/llvm/ADT/Optional.h:176: T* llvm::Optional<T>::getPointer() [with T = llvm::MemoryLocation]: Assertion `Storage.hasVal' failed.</div></div><div><br></div><div><pre style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap">static bool eliminateStrlen(CallInst *CI, BasicBlock::iterator &BBI,
                            AliasAnalysis *AA, MemoryDependenceResults *MD,
                            const DataLayout &DL, const TargetLibraryInfo *TLI,
                            InstOverlapIntervalsTy &IOL,
                            DenseMap<Instruction *, size_t> *InstrOrdering) {

  // Must be a strlen.
  LibFunc Func;
  Function *Callee = CI->getCalledFunction();
  if (!TLI->getLibFunc(*Callee, Func) || !TLI->has(Func) ||
      Func != LibFunc_strlen)
    return false;

  Value *Dst = CI->getOperand(0);
  Instruction *UnderlyingPointer = dyn_cast<Instruction>(GetUnderlyingObject(Dst, DL));
  if (!UnderlyingPointer)
    return false;
  if (isStringFromCalloc(Dst, TLI))
    return false;
  errs() << "before\n";
  if (memoryIsNotModifiedBetween(UnderlyingPointer, CI, AA)) { <--- CRASH
    errs() << "after\n";
  }
  return false;
}</pre><pre style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap">Do you know what is wrong here? I followed the "example" (in eliminateNoopStore) how to use "memoryIsNotModifiedBetween".</pre><pre style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap"><br></pre><pre style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap">Thank you for advice</pre><pre style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap"><br></pre></div></div><div class="gmail_extra"><br><div class="gmail_quote">2018-05-21 21:06 GMT+02:00 Friedman, Eli <span dir="ltr"><<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div text="#000000" bgcolor="#FFFFFF">
    <div class="m_7569404264700124646moz-cite-prefix">memoryIsNotModifiedBetween is precisely
      the sort of expensive walk we shouldn't be doing... I'm surprised
      it hasn't caused any serious issues yet.  Ideally, what we should
      be doing is using MemorySSA to find a dependency from the memset:
      if the closest dependency is the malloc, there aren't any stores
      between the memset and the malloc.  (But we aren't using MemorySSA
      in DSE yet; see <a class="m_7569404264700124646moz-txt-link-freetext" href="https://reviews.llvm.org/D40480" target="_blank">https://reviews.llvm.org/<wbr>D40480</a>.)<br>
      <br>
      But yes, memoryIsNotModifiedBetween has the right meaning.<span class="HOEnZb"><font color="#888888"><br>
      <br>
      -Eli</font></span><div><div class="h5"><br>
      <br>
      On 5/21/2018 7:48 AM, Dávid Bolvanský wrote:<br>
    </div></div></div><div><div class="h5">
    <blockquote type="cite">
      <div dir="ltr"><span style="font-size:12.8px">"memory accesses
          between the malloc and the memset</span><span style="font-size:12.8px"> without an expensive linear scan of
          the block/function" </span>
        <div><span style="font-size:12.8px"><br>
          </span></div>
        <div><span style="font-size:12.8px">(1) do you mean just use
            "memoryIsNotModifiedBetween" function in DSE to check it?</span>
          <div><br>
          </div>
          <div>x = maloc(..);</div>
          <div>memset(x, ...) </div>
          <div><br>
          </div>
          <div>(2) GetUnderlyingObject would give me Value * (from
            malloc) ?</div>
          <div>
            <div><span style="font-size:12.8px"><br>
              </span></div>
            <div><span style="font-size:12.8px">Also another case:</span></div>
            <div><span style="font-size:12.8px"><br>
              </span></div>
            <div><span style="font-size:12.8px">memset(s, 0, len); //
                len > 1</span></div>
            <div><span style="font-size:12.8px">return strlen(s); //
                optimize to 0</span></div>
            <div><span style="font-size:12.8px"><br>
              </span></div>
            <div><span style="font-size:12.8px">(3) How to check
                memset and strlen pairs? I have a strlen call, I have a
                "Value *" for "s". What is the best way to construct
                memset + strlen pairs?</span></div>
            <div><span style="font-size:12.8px"><br>
              </span></div>
            <div><span style="font-size:12.8px">(4) Can this be somehow
                generalized (and not only for strlen)? So
                GetStringLength in ValueTracking would be taught about
                this info (string is empty after memset)</span></div>
            <div><span style="font-size:12.8px"><br>
              </span></div>
            <div><span style="font-size:12.8px">(5) new malloc
                memset folding /  memset + strlen case should be
                implemented in DSE, right?</span></div>
          </div>
        </div>
      </div>
      <div class="gmail_extra"><br>
        <div class="gmail_quote">2018-05-17 21:36 GMT+02:00 Friedman,
          Eli <span dir="ltr"><<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span>:<br>
          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <div text="#000000" bgcolor="#FFFFFF">
              <div class="m_7569404264700124646m_6141790905467921389moz-cite-prefix">The
                fundamental problem with trying to do that sort of
                transform in instcombine is that you don't have access
                to MemoryDependenceAnalysis or MemorySSA; you need a
                data structure like that to figure out whether there are
                any memory accesses between the malloc and the memset
                without an expensive linear scan of the block/function. 
                (You can sort of get around the problem in simple cases
                by adding arbitrary limits to the number of you scan,
                but it doesn't generalize well.)<span class="m_7569404264700124646HOEnZb"><font color="#888888"><br>
                    <br>
                    -Eli</font></span>
                <div>
                  <div class="m_7569404264700124646h5"><br>
                    <br>
                    On 5/17/2018 12:17 PM, Dávid Bolvanský wrote:<br>
                  </div>
                </div>
              </div>
              <div>
                <div class="m_7569404264700124646h5">
                  <blockquote type="cite">
                    <div dir="ltr">As we talked in <a href="https://reviews.llvm.org/D45344" target="_blank">https://reviews.llvm.org/D4534<wbr>4</a>,
                      the problem was dead stores. And I know why :D
                      There was just -instcombine pass. I forgot to do
                      -dse before -instcombine so this is why I did
                      custom "store removal" code there.
                      <div><br>
                      </div>
                      <div>I would like to finish malloc + llvm.memset
                        folding. Yes, you told you would like to see the
                        whole foldMallocMemset in DSE but extend it for
                        llvm.memset in InstCombine... is it really so
                        bad to do? </div>
                      <div>We have standard malloc + memset folding
                        there, so a  few  new lines should not do bad
                        things.</div>
                      <div><br>
                      </div>
                      <div>If I reopen D45344, reupload patch with
                        removed my custom "store removal" code, It could
                        be ok, no? The patch as is worked/works for me
                        for malloc + llvm.memset folding, I would just
                        add -dse to tests to handle dead stores.<br>
                        <div><br>
                        </div>
                        <div><br>
                        </div>
                      </div>
                    </div>
                    <div class="gmail_extra"><br>
                      <div class="gmail_quote">2018-05-17 21:00
                        GMT+02:00 Friedman, Eli <span dir="ltr"><<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span>:<br>
                        <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On 5/17/2018
                            8:58 AM, Dávid Bolvanský via llvm-dev wrote:<br>
                            <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Hello,<br>
                              <br>
                              I would like to find a way to do this
                              removal properly. I found DSE and
                              "eliminateNoopStore" can be useful for
                              this thing.<br>
                              <br>
                              What I mean?<br>
                              int *test = malloc(15 * sizeof(int));<br>
                              test[10] = 12; < ----- remove this
                              store<br>
                              memset(test,0,sizeof(int) * 15);<br>
                            </blockquote>
                            <br>
                          </span> This is classic dead store
                          elimination, and it's already handled by DSE.
                          At least, we optimize the following testcase:<br>
                          <br>
                          #include <stdlib.h><br>
                          #include <string.h><br>
                          void bar(int*);<br>
                          void f() {<span><br>
                              int *test = malloc(15 * sizeof(int));<br>
                              test[10] = 12;<br>
                          </span>   memset(test,0,sizeof(int) * 15);<br>
                            bar(test);<br>
                          }<br>
                          <br>
                          You should be able to look at the existing
                          code to understand how it's handled (using
                          MemoryDependenceAnalysis).<span class="m_7569404264700124646m_6141790905467921389HOEnZb"><font color="#888888"><br>
                              <br>
                              -Eli<br>
                              <br>
                              -- <br>
                              Employee of Qualcomm Innovation Center,
                              Inc.<br>
                              Qualcomm Innovation Center, Inc. is a
                              member of Code Aurora Forum, a Linux
                              Foundation Collaborative Project<br>
                              <br>
                            </font></span></blockquote>
                      </div>
                      <br>
                    </div>
                  </blockquote>
                  <p><br>
                  </p>
                  <pre class="m_7569404264700124646m_6141790905467921389moz-signature" cols="72">-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</pre>
                </div>
              </div>
            </div>
          </blockquote>
        </div>
        <br>
      </div>
    </blockquote>
    <p><br>
    </p>
    <pre class="m_7569404264700124646moz-signature" cols="72">-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</pre>
  </div></div></div>

</blockquote></div><br></div>