src/IceInstX86BaseImpl.h - Issue 1341423002: Reflow comments to use the full width.

Unified Diff: src/IceInstX86BaseImpl.h

Issue 1341423002: Reflow comments to use the full width. (Closed) Base URL: https://chromium.googlesource.com/native_client/pnacl-subzero.git@master

Patch Set: Created 5 years, 3 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: src/IceInstX86BaseImpl.h

diff --git a/src/IceInstX86BaseImpl.h b/src/IceInstX86BaseImpl.h

index 336e268e45e0d1e90a283e3acee9b2586074fdc7..677a1d33563f2b7967e9e460e517139fed935afc 100644

--- a/src/IceInstX86BaseImpl.h

+++ b/src/IceInstX86BaseImpl.h

@@ -112,16 +112,14 @@ InstX86Br<Machine>::InstX86Br(

template <class Machine>

bool InstX86Br<Machine>::optimizeBranch(const CfgNode *NextNode) {

- // If there is no next block, then there can be no fallthrough to

- // optimize.

+ // If there is no next block, then there can be no fallthrough to optimize.

if (NextNode == nullptr)

return false;

// Intra-block conditional branches can't be optimized.

if (Label)

return false;

- // If there is no fallthrough node, such as a non-default case label

- // for a switch instruction, then there is no opportunity to

- // optimize.

+ // If there is no fallthrough node, such as a non-default case label for a

+ // switch instruction, then there is no opportunity to optimize.

if (getTargetFalse() == nullptr)

return false;

@@ -132,15 +130,15 @@ bool InstX86Br<Machine>::optimizeBranch(const CfgNode *NextNode) {

this->setDeleted();

return true;

}

- // If the fallthrough is to the next node, set fallthrough to nullptr

- // to indicate.

+ // If the fallthrough is to the next node, set fallthrough to nullptr to

+ // indicate.

if (getTargetFalse() == NextNode) {

TargetFalse = nullptr;

return true;

}

- // If TargetTrue is the next node, and TargetFalse is not nullptr

- // (which was already tested above), then invert the branch

- // condition, swap the targets, and set new fallthrough to nullptr.

+ // If TargetTrue is the next node, and TargetFalse is not nullptr (which was

+ // already tested above), then invert the branch condition, swap the targets,

+ // and set new fallthrough to nullptr.

if (getTargetTrue() == NextNode) {

assert(Condition != InstX86Base<Machine>::Traits::Cond::Br_None);

Condition = this->getOppositeCondition(Condition);

@@ -185,8 +183,8 @@ InstX86Cmov<Machine>::InstX86Cmov(

typename InstX86Base<Machine>::Traits::Cond::BrCond Condition)

: InstX86Base<Machine>(Func, InstX86Base<Machine>::Cmov, 2, Dest),

Condition(Condition) {

- // The final result is either the original Dest, or Source, so mark

- // both as sources.

+ // The final result is either the original Dest, or Source, so mark both as

+ // sources.

this->addSource(Dest);

this->addSource(Source);

}

@@ -320,12 +318,11 @@ InstX86Fstp<Machine>::InstX86Fstp(Cfg *Func, Variable *Dest)

template <class Machine>

InstX86Pop<Machine>::InstX86Pop(Cfg *Func, Variable *Dest)

: InstX86Base<Machine>(Func, InstX86Base<Machine>::Pop, 0, Dest) {

- // A pop instruction affects the stack pointer and so it should not

- // be allowed to be automatically dead-code eliminated. (The

- // corresponding push instruction doesn't need this treatment

- // because it has no dest variable and therefore won't be dead-code

- // eliminated.) This is needed for late-stage liveness analysis

- // (e.g. asm-verbose mode).

+ // A pop instruction affects the stack pointer and so it should not be

+ // allowed to be automatically dead-code eliminated. (The corresponding push

+ // instruction doesn't need this treatment because it has no dest variable

+ // and therefore won't be dead-code eliminated.) This is needed for

+ // late-stage liveness analysis (e.g. asm-verbose mode).

this->HasSideEffects = true;

}

@@ -529,11 +526,10 @@ void InstX86Jmp<Machine>::emitIAS(const Cfg *Func) const {

Asm->jmp(InstX86Base<Machine>::Traits::RegisterSet::getEncodedGPR(

Var->getRegNum()));

} else {

- // The jmp instruction with a memory operand should be possible

- // to encode, but it isn't a valid sandboxed instruction, and

- // there shouldn't be a register allocation issue to jump

- // through a scratch register, so we don't really need to bother

- // implementing it.

+ // The jmp instruction with a memory operand should be possible to

+ // encode, but it isn't a valid sandboxed instruction, and there

+ // shouldn't be a register allocation issue to jump through a scratch

+ // register, so we don't really need to bother implementing it.

llvm::report_fatal_error("Assembler can't jmp to memory operand");

}

} else if (const auto Mem = llvm::dyn_cast<

@@ -548,11 +544,10 @@ void InstX86Jmp<Machine>::emitIAS(const Cfg *Func) const {

Asm->jmp(CR);

} else if (const auto Imm = llvm::dyn_cast<ConstantInteger32>(Target)) {

// NaCl trampoline calls refer to an address within the sandbox directly.

- // This is usually only needed for non-IRT builds and otherwise not

- // very portable or stable. Usually this is only done for "calls"

- // and not jumps.

- // TODO(jvoung): Support this when there is a lowering that

- // actually triggers this case.

+ // This is usually only needed for non-IRT builds and otherwise not very

+ // portable or stable. Usually this is only done for "calls" and not jumps.

+ // TODO(jvoung): Support this when there is a lowering that actually

+ // triggers this case.

(void)Imm;

llvm::report_fatal_error("Unexpected jmp to absolute address");

} else {

@@ -633,10 +628,9 @@ void InstX86Call<Machine>::dump(const Cfg *Func) const {

getCallTarget()->dump(Func);

}

-// The ShiftHack parameter is used to emit "cl" instead of "ecx" for

-// shift instructions, in order to be syntactically valid. The

-// this->Opcode parameter needs to be char* and not IceString because of

-// template issues.

+// The ShiftHack parameter is used to emit "cl" instead of "ecx" for shift

+// instructions, in order to be syntactically valid. The this->Opcode parameter

+// needs to be char* and not IceString because of template issues.

template <class Machine>

void InstX86Base<Machine>::emitTwoAddress(const char *Opcode, const Inst *Inst,

const Cfg *Func, bool ShiftHack) {

@@ -802,15 +796,14 @@ void InstX86Base<Machine>::emitIASGPRShift(

&Emitter) {

typename InstX86Base<Machine>::Traits::Assembler *Asm =

Func->getAssembler<typename InstX86Base<Machine>::Traits::Assembler>();

- // Technically, the Dest Var can be mem as well, but we only use Reg.

- // We can extend this to check Dest if we decide to use that form.

+ // Technically, the Dest Var can be mem as well, but we only use Reg. We can

+ // extend this to check Dest if we decide to use that form.

assert(Var->hasReg());

// We cheat a little and use GPRRegister even for byte operations.

typename InstX86Base<Machine>::Traits::RegisterSet::GPRRegister VarReg =

InstX86Base<Machine>::Traits::RegisterSet::getEncodedByteRegOrGPR(

Ty, Var->getRegNum());

- // Src must be reg == ECX or an Imm8.

- // This is asserted by the assembler.

+ // Src must be reg == ECX or an Imm8. This is asserted by the assembler.

if (const auto SrcVar = llvm::dyn_cast<Variable>(Src)) {

assert(SrcVar->hasReg());

typename InstX86Base<Machine>::Traits::RegisterSet::GPRRegister SrcReg =

@@ -1337,8 +1330,8 @@ void InstX86Imul<Machine>::emitIAS(const Cfg *Func) const {

&InstX86Base<Machine>::Traits::Assembler::imul};

emitIASOpTyGPR<Machine>(Func, Ty, this->getSrc(1), Emitter);

} else {

- // We only use imul as a two-address instruction even though

- // there is a 3 operand version when one of the operands is a constant.

+ // We only use imul as a two-address instruction even though there is a 3

+ // operand version when one of the operands is a constant.

assert(Var == this->getSrc(0));

static const typename InstX86Base<

Machine>::Traits::Assembler::GPREmitterRegOp Emitter = {

@@ -1678,8 +1671,8 @@ void InstX86Cmpps<Machine>::emitIAS(const Cfg *Func) const {

Func->getAssembler<typename InstX86Base<Machine>::Traits::Assembler>();

assert(this->getSrcSize() == 2);

assert(Condition < InstX86Base<Machine>::Traits::Cond::Cmpps_Invalid);

- // Assuming there isn't any load folding for cmpps, and vector constants

- // are not allowed in PNaCl.

+ // Assuming there isn't any load folding for cmpps, and vector constants are

+ // not allowed in PNaCl.

assert(llvm::isa<Variable>(this->getSrc(1)));

const auto SrcVar = llvm::cast<Variable>(this->getSrc(1));

if (SrcVar->hasReg()) {

@@ -1988,8 +1981,8 @@ void InstX86Ucomiss<Machine>::emit(const Cfg *Func) const {

template <class Machine>

void InstX86Ucomiss<Machine>::emitIAS(const Cfg *Func) const {

assert(this->getSrcSize() == 2);

- // Currently src0 is always a variable by convention, to avoid having

- // two memory operands.

+ // Currently src0 is always a variable by convention, to avoid having two

+ // memory operands.

assert(llvm::isa<Variable>(this->getSrc(0)));

const auto Src0Var = llvm::cast<Variable>(this->getSrc(0));

Type Ty = Src0Var->getType();

@@ -2291,16 +2284,16 @@ template <class Machine> void InstX86Mov<Machine>::emit(const Cfg *Func) const {

: InstX86Base<Machine>::Traits::TypeAttributes[DestTy]

.SdSsString) << "\t";

}

- // For an integer truncation operation, src is wider than dest.

- // Ideally, we use a mov instruction whose data width matches the

- // narrower dest. This is a problem if e.g. src is a register like

- // esi or si where there is no 8-bit version of the register. To be

- // safe, we instead widen the dest to match src. This works even

- // for stack-allocated dest variables because typeWidthOnStack()

- // pads to a 4-byte boundary even if only a lower portion is used.

- // TODO: This assert disallows usages such as copying a floating point

- // value between a vector and a scalar (which movss is used for).

- // Clean this up.

+ // For an integer truncation operation, src is wider than dest. Ideally, we

+ // use a mov instruction whose data width matches the narrower dest. This is

+ // a problem if e.g. src is a register like esi or si where there is no 8-bit

+ // version of the register. To be safe, we instead widen the dest to match

+ // src. This works even for stack-allocated dest variables because

+ // typeWidthOnStack() pads to a 4-byte boundary even if only a lower portion

+ // is used.

+ // TODO: This assert disallows usages such as copying a floating

+ // point value between a vector and a scalar (which movss is used for). Clean

+ // this up.

assert(Func->getTarget()->typeWidthInBytesOnStack(DestTy) ==

Func->getTarget()->typeWidthInBytesOnStack(SrcTy));

Src->emit(Func);

@@ -2316,12 +2309,11 @@ void InstX86Mov<Machine>::emitIAS(const Cfg *Func) const {

Type DestTy = Dest->getType();

Type SrcTy = Src->getType();

// Mov can be used for GPRs or XMM registers. Also, the type does not

- // necessarily match (Mov can be used for bitcasts). However, when

- // the type does not match, one of the operands must be a register.

- // Thus, the strategy is to find out if Src or Dest are a register,

- // then use that register's type to decide on which emitter set to use.

- // The emitter set will include reg-reg movs, but that case should

- // be unused when the types don't match.

+ // necessarily match (Mov can be used for bitcasts). However, when the type

+ // does not match, one of the operands must be a register. Thus, the strategy

+ // is to find out if Src or Dest are a register, then use that register's

+ // type to decide on which emitter set to use. The emitter set will include

+ // reg-reg movs, but that case should be unused when the types don't match.

static const typename InstX86Base<Machine>::Traits::Assembler::XmmEmitterRegOp

XmmRegEmitter = {&InstX86Base<Machine>::Traits::Assembler::movss,

&InstX86Base<Machine>::Traits::Assembler::movss};

@@ -2333,16 +2325,16 @@ void InstX86Mov<Machine>::emitIAS(const Cfg *Func) const {

Machine>::Traits::Assembler::GPREmitterAddrOp GPRAddrEmitter = {

&InstX86Base<Machine>::Traits::Assembler::mov,

&InstX86Base<Machine>::Traits::Assembler::mov};