<HTML><BODY style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space; "><DIV><BLOCKQUOTE type="cite"><DIV><BLOCKQUOTE type="cite"><DIV><BR class="khtml-block-placeholder"></DIV><DIV>This adds an extra word to LoadInst and StoreInst. Since alignments must be a power of two, I suggest representing them as a shift amount (e.g, an alignment of 16 is stored as 4, 1<<4 == 16). Given this, we don't need a large number of bits to support very large shift amounts. Given this, you can store it in the "SubClassData" member of Value (which is 16 bits wide), to avoid using the word. Load and StoreInst currently store a bit in that word. The only trick here is that we want power-of-two-plus one, so that "0" represents an alignment of zero, not an alignment of 1.</DIV></BLOCKQUOTE><DIV><BR class="khtml-block-placeholder"></DIV><DIV>Yeah, I thought about that. It's not a bit problem, I'll encode the alignment the same way it's encoded in the BC which has this same issue.</DIV></DIV></BLOCKQUOTE><DIV><BR class="khtml-block-placeholder"></DIV><DIV>sounds good.</DIV><BR><BLOCKQUOTE type="cite"><DIV><BLOCKQUOTE type="cite"><DIV></DIV><DIV>Please don't <FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;">#include MathExtras.h in Instructions.h</SPAN></FONT></DIV></BLOCKQUOTE><DIV><BR class="khtml-block-placeholder"></DIV><DIV>No prob. I'll have to move the accessor methods to the .cpp file and include MathExtras there to use the Log2_32 function, though.</DIV></DIV></BLOCKQUOTE><DIV><BR class="khtml-block-placeholder"></DIV><DIV>Ok. The "getter" should be inlined (it just uses <<, nothing exotic like log :).</DIV><BR><BLOCKQUOTE type="cite"><DIV><BLOCKQUOTE type="cite"><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;">+</SPAN></FONT><FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;"> </SPAN></FONT><FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;">LoadInst(Value *Ptr, const std::string &Name, bool isVolatile = false,</SPAN></FONT><FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;"> </SPAN></FONT></DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; "><FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;">+ </SPAN></FONT><FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;"> </SPAN></FONT><FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;">Instruction *InsertBefore = 0, unsigned Align = 0);</SPAN></FONT></DIV><DIV><FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;"><BR class="khtml-block-placeholder"></SPAN></FONT></DIV><DIV><FONT class="Apple-style-span" face="Monaco" size="2"><SPAN class="Apple-style-span" style="font-size: 10px;">The instruction* to insert before should be the last member. In the case of alignment, I don't think we need a helper constructor for this (in fact, we probably don't need one for volatile either). Any clients that want to create a load instruction with a specific alignment can create it, then use setAlignment() :)</SPAN></FONT></DIV></BLOCKQUOTE><DIV><BR class="khtml-block-placeholder"></DIV><DIV>These helper constructors were used quite prolifically in the lowering passes, like the hunk below. Do you prefer them to be changed to set the attribute directly?</DIV></DIV></BLOCKQUOTE><DIV><BR class="khtml-block-placeholder"></DIV><DIV>All of the codegen stuff looks good and should stay as you wrote it, I'm just talking about the LLVM level stuff. If you think that having an alignment-taking-ctor is useful, feel free to keep it, but move the instruction-to-insert-before to the end of the operand list.</DIV><DIV><BR class="khtml-block-placeholder"></DIV><DIV>-Chris</DIV><BR></DIV><BR></BODY></HTML>