Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

better loop support #36

Open
Trass3r opened this issue Sep 19, 2019 · 8 comments
Open

better loop support #36

Trass3r opened this issue Sep 19, 2019 · 8 comments

Comments

@Trass3r
Copy link
Contributor

Trass3r commented Sep 19, 2019

Loops usually generate some inc/dec instructions instead of add when -Os is used:

int sum(int* arr, int n) {
	int sum = 0;
	for (int i = 0; i < n; ++i)
		sum += arr[i];
	return sum;
}
@bharadwajy
Copy link
Contributor

Thank you very much!

Would really appreciate if you could create PR once you are happy with your changes. Please add C test sources to test/smoke_test to verify the changes proposed. I'll be happy to pull the changes up to master.

@Trass3r
Copy link
Contributor Author

Trass3r commented Sep 19, 2019

Btw what is the reason for that huge map? MCInst doesn't contain the width information?

@bharadwajy
Copy link
Contributor

Btw what is the reason for that huge map? MCInst doesn't contain the width information?

Not at the time when I started the work. I did not see anything that indicates things have changed in that respect.

@Trass3r
Copy link
Contributor Author

Trass3r commented Sep 19, 2019

Hmm odd how do they represent something like mov DWORD PTR [rbp-0x4], edi then?

@Trass3r
Copy link
Contributor Author

Trass3r commented Sep 20, 2019

INC on memory is tricky though: inc DWORD PTR [rsp+rax*4-0x38]
Fails in

assert((SrcOp.isImm() || SrcOp.isReg()) &&
.
Operand 6 is "Metadata", 7 is register.
If I add

  if (MI.getOperand(SrcOpIndex).isMetadata())
    ++SrcOpIndex;

I get "Super register not found"
Any ideas?

@bharadwajy
Copy link
Contributor

bool X86MachineInstructionRaiser::raiseMoveToMemInstr() is implemented to raise an instruction that moves to memory the content of an explicit source register or immediate value. So, that function is not designed to raise INCXXm instructions.

Support for INCXXm instructions is not yet implemented. That said, take a look at bool X86MachineInstructionRaiser::raiseNotOpMemInstr(). Basically that function generates an instruction to load from memory location, an operation (specifically, not) instruction and an instruction to store to the memory location. This may be an opportunity to expand the class NOT_OP_MEM (to say INPLACE_MEM_OP) to raise instructions that load, operate and store.

@bharadwajy
Copy link
Contributor

Btw what is the reason for that huge map? MCInst doesn't contain the width information?

Not at the time when I started the work. I did not see anything that indicates things have changed in that respect.

However, eventually, I really want to get rid of the burden of maintaining this table and replace its services using LLVM APIs or some other tblgen based utility.

@Trass3r
Copy link
Contributor Author

Trass3r commented Sep 25, 2019

This may be an opportunity to expand the class NOT_OP_MEM (to say INPLACE_MEM_OP) to raise instructions that load, operate and store.

Thanks that helped! Trass3r@f100b2e

I really want to get rid of the burden of maintaining this table and replace its services using LLVM APIs or some other tblgen based utility.

I experimented a bit with that (see my repo) but I think it would require some refactoring of the raise functions into something more generic working on general a = b op c instructions with common and transparent handling of reg/mem/imm operands.

Trass3r added a commit to Trass3r/llvm-mctoll that referenced this issue Sep 26, 2019
generalizing the code for NOT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants