<div dir="ltr"><br><br><div class="gmail_quote">On Fri Feb 13 2015 at 4:00:39 PM reed kotler <<a href="mailto:rkotler@mips.com">rkotler@mips.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    <div>Subtarget is initialized first and then
      TargetSupported.<br>
      <br>
      It may have been a bad choice of name but TargetSupported is
      derived from Subtarget.<br>
      That name has been in Mips fast-isel for a long time now. It's a
      local variable in the mips fast-isel class.<br>
      <br>
      Probably should have been called SubtargetSupported.</div></div><div bgcolor="#FFFFFF" text="#000000"><div><br>
      <br></div></div></blockquote><div><br></div><div>Rather than write it out again, how about you look at the patch that changed the code that you're trying to change back?</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><div>
      On 02/13/2015 03:50 PM, Eric Christopher wrote:<br>
    </div></div><div bgcolor="#FFFFFF" text="#000000">
    <blockquote type="cite">
      
      <div dir="ltr"><br>
        <br>
        <div class="gmail_quote">On Fri Feb 13 2015 at 3:47:23 PM reed
          kotler <<a href="mailto:rkotler@mips.com" target="_blank">rkotler@mips.com</a>>
          wrote:<br>
          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Not sure I
            understand some of these comments.<br>
            <br>
            On 02/13/2015 03:21 PM, Eric Christopher wrote:<br>
            > Hi Reed,<br>
            ><br>
            > Some quick inline comments.<br>
            ><br>
            > +      assert(isFIBase() && "Invalid base frame
            index  access!");<br>
            ><br>
            > formatting.<br>
            This how clang format does it. What do you want?<br>
          </blockquote>
          <div><br>
          </div>
          <div>You have an extra space between index and access.</div>
          <div> </div>
          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
            > - const MipsSubtarget *Subtarget; const TargetInstrInfo
            &TII; const TargetLowering &TLI;<br>
            ><br>
            > +  const MipsSubtarget *Subtarget;<br>
            ><br>
            > Unnecessary.<br>
            ><br>
            > +        Subtarget(&TM.getSubtarget<MipsSubtarget>())
            {<br>
            ><br>
            > No.<br>
            ><br>
            > +    TargetSupported = ((Subtarget->getRelocationModel()
            == Reloc::PIC_) &&<br>
            > +                       ((Subtarget->hasMips32r2()
            ||<br>
            > Subtarget->hasMips32()) &&<br>
            > +                        (Subtarget->isABI_O32())));<br>
            ><br>
            > Ditto.<br>
            <br>
            What is the issue here?<br>
            <br>
          </blockquote>
          <div><br>
          </div>
          <div>You've moved where subtarget is initialized, and changed
            (in the wrong way) how Subtarget and TargetSupported are
            initialized.</div>
          <div> </div>
          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
            Formatting?<br>
            <br>
            This is what clang format does.<br>
            ><br>
            > +  // This code is mostly cloned from AArch64 (which
            cloned it from earlier<br>
            > +  // ports)'<br>
            ><br>
            > Unnecessary.<br>
            You mean the comment is not necessary?<br>
          </blockquote>
          <div><br>
          </div>
          <div>Correct.</div>
          <div> </div>
          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
            ><br>
            > +    unsigned Offset = Addr.getOffset();<br>
            > +    ;<br>
            > +    MachineFrameInfo &MFI =
            *MF->getFrameInfo();<br>
            > +    ;<br>
            ><br>
            > ?<br>
            You mean the extra ";" lines?<br>
            Hmmm.. probably from clang format but I don't know.<br>
          </blockquote>
          <div><br>
          </div>
          <div>clang-format would not have put them there. It doesn't
            insert characters.</div>
          <div> </div>
          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
            Looks wrong.<br>
          </blockquote>
          <div><br>
          </div>
          <div>Yep.</div>
          <div><br>
          </div>
          <div>-eric</div>
          <div> </div>
          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <br>
            > -eric<br>
            ><br>
            ><br>
            > <a href="http://reviews.llvm.org/D6426" target="_blank">http://reviews.llvm.org/D6426</a><br>
            ><br>
            > EMAIL PREFERENCES<br>
            >    <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
            ><br>
            ><br>
            <br>
            _______________________________________________<br>
            llvm-commits mailing list<br>
            <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
            <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
          </blockquote>
        </div>
      </div>
    </blockquote>
    <br>
  </div></blockquote></div></div>